Closed Bug 1287717 Opened 3 years ago Closed 3 years ago

[Presentation WebAPI] PresentationConnectionClosedEvent is not fired on controller page when the MIME type of receiver page is not supported

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla51
blocking-b2g 2.6+
Tracking Status
b2g-v2.6 --- fixed
firefox51 --- fixed

People

(Reporter: twen, Assigned: schien)

References

Details

Attachments

(3 files, 3 obsolete files)

When the presentation URL is a pdf file, TV receives a blank page and the controller terminates connection. According to the W3C spec: https://www.w3.org/TR/presentation-api/#starting-a-presentation 
  21. If any of the following steps fails, abort all remaining steps and close the presentation connection S with error as closeReason, and a human readable message describing the failure as closeMessage.

The connection should be closed with error message.

STR:
1. Use the sample web page: https://people.mozilla.org/~schien/test-presentation.html
and change the presentation URL to a pdf file
2. Use Fennec to open the web page

Expected Behavior:
Show error with error message

Actual Behavior:
Show error message on controller, and possibly close presentation on receiver side
blocking-b2g: --- → 2.6?
**Correction**
Actual Behavior:
Blank page on receiver, connection terminated on controller side
Two things need to be done.

1. We will need to change the state transition from "terminated" to "closed" in controller side.
https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#769

2. We will need to close the receiver window if loading timeout is detected.
https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#1177
Summary: [Presentation WebAPI] Pdf url with no error → [Presentation WebAPI] PresentationConnectionClosedEvent is not fired on controller page when the MIME type of receiver page is not supported
Assignee: nobody → schien
Attachment #8775025 - Flags: review?(bugs)
Comment on attachment 8775025 [details] [diff] [review]
part 1, enter-closed-state-while-fail-to-connect.patch

ah, "closed means that the presentation connection has been closed, or could not be opened."
Attachment #8775025 - Flags: review?(bugs) → review+
blocking-b2g: 2.6? → 2.6+
For unreachable URL and unsupported MIME type, we can monitor the STATE_STOP since there is no STATE_TRANSFERRING.
@smaug, do you know if there is any way to test loading unreachable URL and unsupported MIME type in mochitest? I tried changing header with the ^header^ file but still get STATE_TRANSFERRING.
Attachment #8776541 - Flags: feedback?(bugs)
Didn't I just review something related to that... it was using information about the popup firefox has when unknown mime type is being loaded.
Yes, https://bugzilla.mozilla.org/show_bug.cgi?id=1282407.
Would that setup work here for testing?
or perhaps other tests testing download attribute?
found the correct way to use ^headers^ file, update patch with test cases.
Attachment #8776541 - Attachment is obsolete: true
Attachment #8776541 - Flags: feedback?(bugs)
Attachment #8776850 - Flags: review?(bugs)
Comment on attachment 8775025 [details] [diff] [review]
part 1, enter-closed-state-while-fail-to-connect.patch

>   if (NS_WARN_IF(NS_FAILED(aReason) || !mIsResponderReady)) {
>     // The presentation session instance may already exist.
>     // Change the state to TERMINATED since it never succeeds.
>-    SetStateWithReason(nsIPresentationSessionListener::STATE_TERMINATED, aReason);
>+    if (nsIPresentationSessionListener::STATE_TERMINATED != mState) {
>+      SetStateWithReason(nsIPresentationSessionListener::STATE_CLOSED, aReason);
>+    }

Actually, the comment here is wrong, since it talks about "TERMINATED"
Attachment #8776850 - Flags: review?(bugs) → review+
update according to comment #9, carry r+.
Attachment #8775025 - Attachment is obsolete: true
Attachment #8777170 - Flags: review+
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): presentation api
User impact if declined: receiver page will not be closed when load failure
Testing completed: with mochitest
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: n/a
Attachment #8777243 - Flags: approval-mozilla-b2g48?
has problems to apply: renamed 1287717 -> close-receiver-page-while-timeout.patch
applying close-receiver-page-while-timeout.patch
patching file dom/presentation/tests/mochitest/mochitest.ini
Hunk #1 FAILED at 13
1 out of 2 hunks FAILED -- saving rejects to file dom/presentation/tests/mochitest/mochitest.ini.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh close-receiver-page-while-timeout.patch
Flags: needinfo?(schien)
Keywords: checkin-needed
resolve merge conflict, carry r+.
Attachment #8776850 - Attachment is obsolete: true
Flags: needinfo?(schien)
Attachment #8777638 - Flags: review+
(In reply to Carsten Book [:Tomcat] from comment #13)
> has problems to apply: renamed 1287717 ->
> close-receiver-page-while-timeout.patch
> applying close-receiver-page-while-timeout.patch
> patching file dom/presentation/tests/mochitest/mochitest.ini
> Hunk #1 FAILED at 13
> 1 out of 2 hunks FAILED -- saving rejects to file
> dom/presentation/tests/mochitest/mochitest.ini.rej
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working directory
> errors during apply, please fix and qrefresh
> close-receiver-page-while-timeout.patch

resolved with new part 2 patch uploaded.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/025466d995ef
Part 1, enter closed state while fail to connect. r=smaug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/734a7d13d285
Part 2, close receiver page while loading fail. r=smaug.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/025466d995ef
https://hg.mozilla.org/mozilla-central/rev/734a7d13d285
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8777243 [details] [review]
pull request for tv 2.6

Approve for TV 2.6
Attachment #8777243 - Flags: approval-mozilla-b2g48? → approval-mozilla-b2g48+
Comment on attachment 8777243 [details] [review]
pull request for tv 2.6

ni? @xeonchen for uplift
Flags: needinfo?(xeonchen)
Duplicate of this bug: 1287722
Duplicate of this bug: 1246060
Hi SC,

Can you confirm the fixed behavior is as described in comment 2:
1. Close connection.
2. Close blank receiver window.
Flags: needinfo?(schien)
(In reply to Teri Wen [:twen] from comment #23)
> Hi SC,
> 
> Can you confirm the fixed behavior is as described in comment 2:
> 1. Close connection.
> 2. Close blank receiver window.

Yes, the behavior should be matched with your description. Did you see different behavior?
Flags: needinfo?(schien)
Everything looks right, just want to make sure I don't miss anything :)

Verified fixed.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.