Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make about:srcdoc to inherit PolicyContainer from history. #8057

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

ArthurSonzogni
Copy link
Member

@ArthurSonzogni ArthurSonzogni commented Jun 30, 2022

This is the current behavior of Chrome and Firefox. Safari do not support history navigations at all.

See WPT.
https://wpt.fyi/results/content-security-policy/inheritance/iframe-srcdoc-history-inheritance.html?label=experimental&label=master&aligned

I would like to align the specification and WPTs to match the current browsers behaviors.
See: #6809

(See WHATWG Working Mode: Changes for more details.)


/origin.html ( diff )

This is the current behavior of Chrome and Firefox:

See WPT.
https://wpt.fyi/results/content-security-policy/inheritance/iframe-srcdoc-history-inheritance.html?label=experimental&label=master&aligned

I would like to update the specification and the WPT test to align with
the current browsers behaviors.

See: whatwg#6809
@ArthurSonzogni
Copy link
Member Author

Question:
What kind of request for position would you like to see for Chrome/Firefox for aligning the specification with their behavior? What about Safari, who don't have about:srcdoc history navigations (so far)?

@antosart FYI: in case I am missing some changes in the spec.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 30, 2022
This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug:1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Thanks!

In general you can assume that browsers which already implement a change, support implementing that change :). So you can check that box.

It would be good to submit a WebKit bug, perhaps for the general area of srcdoc history navigation, with a pointer to any relevant web platform tests. (Especially ones which pass in the two other engines.)

I will wait for @antosart to double-check before merging.

@domenic
Copy link
Member

domenic commented Jul 4, 2022

Would you be able to add tests for the other things in the policy container, BTW? Referrer policy at least seems easy enough to test. Maybe embedder policy is not easy to test or does not give a different result anyway due to the consistency guarantees for the frame...

@ArthurSonzogni
Copy link
Member Author

Would you be able to add tests for the other things in the policy container, BTW? Referrer policy at least seems easy enough to test. Maybe embedder policy is not easy to test or does not give a different result anyway due to the consistency guarantees for the frame...

Yes. Good idea! I remember I already reviewed @antosart adding a test for this (Thanks!):
https://wpt.fyi/results/referrer-policy/generic/inheritance/iframe-inheritance-history-about-srcdoc.html?

The test is expecting the new proposed behavior.

That's very interesting. We see Firefox doesn't inherit the referrer policy from history, contrary to CSP.
It means Firefox is not consistent with PolicyContainer. This is not surprising, because Firefox doesn't really use the concept of PolicyContainer and every policies are inherited individually.

I guess this require me to ask Firefox their position. I will do next week!

@domenic
Copy link
Member

domenic commented Jul 14, 2022

I think it is OK to treat Firefox's inconsistency here (restoring CSP, and the document itself, but not the referrer policy) as a bug. So we can merge this as-is.

@domenic domenic merged commit 5be9e62 into whatwg:main Jul 14, 2022
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 14, 2022
This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 14, 2022
This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 14, 2022
This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 21, 2022
…iner from history., a=testonly

Automatic update from web-platform-tests
Make about:srcdoc to inherit PolicyContainer from history.

This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}

--

wpt-commits: 5152f7028dd6db37dfaa12f52fa467833cabd8e5
wpt-pr: 34659
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 27, 2022
…iner from history., a=testonly

Automatic update from web-platform-tests
Make about:srcdoc to inherit PolicyContainer from history.

This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}

--

wpt-commits: 5152f7028dd6db37dfaa12f52fa467833cabd8e5
wpt-pr: 34659
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This correspond to HTML PR:
whatwg/html#8057
We should wait for the HTML PR to be merged before reflecting this into
the test.

This makes the test to pass on Chrome and Firefox, who both restore
PolicyContainer from history.

Bug: 1329190
Change-Id: Ib9dfd5a592295cbabb5dffbb690e430c350161ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3735763
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024257}
NOKEYCHECK=True
GitOrigin-RevId: dd835784014c20f5f70f2fb472a29eab6fdfdaa3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants