Skip to content
This repository was archived by the owner on Jul 30, 2019. It is now read-only.

Adds allow-presentation to iframe sandbox flags - #861

Merged
chaals merged 1 commit into
w3c:masterfrom
tidoust:sandboxing-allow-presentation
Apr 5, 2017
Merged

chaals merged 1 commit into
w3c:masterfrom
tidoust:sandboxing-allow-presentation

Conversation

@tidoust

@tidoust tidoust commented Apr 3, 2017

Copy link
Copy Markdown
Member

The Presentation API uses the "allow-presentation" iframe sandbox flag to enable the API in nested browsing contexts. See issue #437.

The Presentation API uses the "allow-resentation" ifram sandbox flag to enable
the API in nested browsing contexts. See issue #437.
@chaals

chaals commented Apr 4, 2017

Copy link
Copy Markdown
Collaborator

The commit looks like it does the right thing - thanks :)

My brief look at the tests referenced in the issue - i.e. I clicked the links - gave me a bit of json saying error, 404 in Firefox, Yandex (blink), and Safari.

Does that mean the test passes, or that something is broken? Do you have a pointer that explains the test results?

@chaals
chaals requested a review from travisleithead April 4, 2017 05:42
@chaals

chaals commented Apr 4, 2017

Copy link
Copy Markdown
Collaborator

Hmm. I found something that said it was a preliminary implementation report suggesting the first two tests aren't implemented and the other two are.

??

@tidoust

tidoust commented Apr 4, 2017

Copy link
Copy Markdown
Member Author

Hi @chaals,

The preliminary implementation report needs to be updated. More importantly, the sandboxing checks have been moved to the PresentationRequest constructor within the Presentation API, so tests have changed in the test suite accordingly. See latest comment in #437.

As mentioned in that other issue, implementation-wise, Google Chrome Canary passes these tests, but the implementation of the Presentation API in Mozilla Firefox for Android has not yet been updated to perform the check on the PresentationRequest constructor.

@travisleithead travisleithead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super! LGTM.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants