1314229 - [Presentation API] presentation.receiver should be null if not in corresponding context

Status: RESOLVED FIXED

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

Description

[Tommy Kuo (away forever...)]

Currently, presentation.receiver is undefined if a page cannot pass `HasReceiverSupport` check. And it should be null.

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

Review commit: https://reviewboard.mozilla.org/r/89786/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89786/

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

https://reviewboard.mozilla.org/r/89786/#review89230

::: dom/webidl/Presentation.webidl:20
(Diff revision 1)
> -  *
> -  * The origin of this IDL file is
> -  * https://w3c.github.io/presentation-api/#interface-presentation
>    */
> -  [Pref="dom.presentation.controller.enabled"]
>    attribute PresentationRequest? defaultRequest;

defaultRequest should be exposed only in controlling side.

::: dom/webidl/Presentation.webidl:26
(Diff revision 1)
>  
>    /*
>     * This should be available on the receiving browsing context in order to
>     * access the controlling browsing context and communicate with them.
>     */
> -  [SameObject,
> +  [SameObject]

this isn't right.
presentation.receiver should be exposed only in the receiver side. 
"Receiving user agents MUST implement the following partial interface: "

and there it should be null expect in case of
"The receiver attribute MUST return the PresentationReceiver instance associated with the receiving browsing context and created by the receiving user agent when the receiving browsing context is created. "

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

https://reviewboard.mozilla.org/r/89786/#review89230

> defaultRequest should be exposed only in controlling side.

Ok, so there is only one rule described in spec, i.e. controlling user agent must implement this interface. Therefore, we use pref "dom.presentation.controller.enabled" to denote the browser as a controlling UA.

On Fennec, since it is 1-UA scenario, it served as both controlling UA and receiving UA. By spec we should still expose `defaultRequest` in receiver browsing context since it is loaded on a browser that supports controlling UA behavior. Is this a correct understanding of the current spec?

> this isn't right.
> presentation.receiver should be exposed only in the receiver side. 
> "Receiving user agents MUST implement the following partial interface: "
> 
> and there it should be null expect in case of
> "The receiver attribute MUST return the PresentationReceiver instance associated with the receiving browsing context and created by the receiving user agent when the receiving browsing context is created. "

So, two rule are described in the spec:
1. Receiving user agent must implement this interface
2. Return null in browsing context other than receiving browsing context

For rule 1 we can use pref "dom.presentation.receiver.enabled" to denote the browser as a receiving UA.
For rule 2 we check if the current page is loaed in a <browser> or <iframe mozbrowser> with `mozpresentation` specified. Moreover, by considering the nested iframe scenario, we further check if the iframe is in the same origin of the requested receiver page (which information stores in `mozpresentation` attribute).

In addition, `receiver` should return null instead of undefined on Fennec on regular web page since Fennec supports receiving UA behavior as well. Is this a correct understanding of the current spec?

Comment 4

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

@smaug see comment #3 (MozReview should provide a UI to ni? people on review comments)

Comment 5

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89786/diff/1-2/

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6f4e255b676

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=15407309473b

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #3)
> In addition, `receiver` should return null instead of undefined on Fennec on
> regular web page since Fennec supports receiving UA behavior as well. Is
> this a correct understanding of the current spec?
If that is the case, then yes.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

https://reviewboard.mozilla.org/r/89786/#review89748

::: dom/presentation/Presentation.cpp:123
(Diff revision 2)
> +  if (!docShell) {
> +    return false;
> +  }
> +
> +  if (!Preferences::GetBool("dom.presentation.testing.simulate-receiver") &&
> +      !docShell->GetIsInMozBrowserOrApp()) {

So this will be updated in the other bug to support also all the top level browsing contexts, right?

::: dom/presentation/tests/mochitest/file_presentation_receiver.html:58
(Diff revision 2)
>      );
>      command({ name: 'trigger-incoming-offer' });
>    });
>  }
>  
> -function testDefauleRequestIsNull() {
> +function testDefauleRequestIsUndefined() {

s/testDefauleRequestIsUndefined/testDefaultRequestIsUndefined/

Comment 10

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

https://reviewboard.mozilla.org/r/89786/#review89748

> So this will be updated in the other bug to support also all the top level browsing contexts, right?

Yes, bug 1305352 needs to rebase after my patch landed.

Comment 11

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8806285 [details]
Bug 1314229 - return undefined/null for defaultRequest/receiver if not in corresponding UA or context. .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89786/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74f6bc91f747
return undefined/null for defaultRequest/receiver if not in corresponding UA or context. r=smaug.

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/74f6bc91f747