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

Coop noopener allow popups #36232

Merged
merged 22 commits into from
Nov 12, 2024
Merged

Conversation

yoavweiss
Copy link
Contributor

Description

Added the new noopener-allow-popup COOP value

Motivation

To inform developers of this new header value.

Additional details

Related issues and pull requests

Fixes mdn/mdn#579
Related to whatwg/html#10394

@yoavweiss yoavweiss requested review from a team as code owners October 7, 2024 11:34
@yoavweiss yoavweiss requested review from hamishwillee and chrisdavidmills and removed request for a team October 7, 2024 11:34
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs size/s [PR only] 6-50 LoC changed labels Oct 7, 2024
Copy link
Contributor

github-actions bot commented Oct 7, 2024

Preview URLs

External URLs (4)

URL: /en-US/docs/Glossary/Browsing_context
Title: Browsing context


URL: /en-US/docs/Web/API/Window/crossOriginIsolated
Title: Window: crossOriginIsolated property


URL: /en-US/docs/Web/API/WorkerGlobalScope/crossOriginIsolated
Title: WorkerGlobalScope: crossOriginIsolated property


URL: /en-US/docs/Web/HTTP/Headers/Cross-Origin-Opener-Policy
Title: Cross-Origin-Opener-Policy

(comment last updated: 2024-11-12 07:02:27)

@github-actions github-actions bot added size/xs [PR only] 0-5 LoC changed and removed Content:HTML Hypertext Markup Language docs size/s [PR only] 6-50 LoC changed labels Oct 7, 2024
@yoavweiss
Copy link
Contributor Author

Apologies, but I botched the commit history on this PR. I think it would be squashed away at merge time, but let me know if that's not the case, and I'll create a new PR instead of this one.

@hamishwillee
Copy link
Collaborator

@yoavweiss Thanks for this. Can you create an update to the browser compatibility data for this? We document things that are released, and I suspect it will be a lot easier for you to work out the version of chrome that support this than me.

I tidied the commit history by dropping the commits that didn't make sense. Squash probably would have worked.

IMO this is not quite sufficient. Specifically I don't think that someone can use this and understand what it is for without reading the spec and the explainer. Further, I think that was true before you made these changes. Consider:

The HTTP Cross-Origin-Opener-Policy (COOP) response header allows you to ensure a top-level document does not share a browsing context group with cross-origin documents.

What's a browsing context group and who cares if they share a cross-origin document?

To fix this:

  1. Explanation of what a browsing context group is, and what problem this COOP header solves - I can help write that if you can point me to an explainer.
  2. Perhaps spell out that no opener of for same-origin too. Coop noopener allow popups #36232 (comment)
  3. An example addressing this particular use case. Using the example of the password/chat app from the explainer would work. Otherwise this particular case is a bit abstract.

Does that make sense?

@yoavweiss
Copy link
Contributor Author

Thanks @hamishwillee ! This makes perfect sense.

What's a browsing context group and who cares if they share a cross-origin document?

I found an explainer that outlines that. Should this be part of this PR? Or a separate one to improve COOP docs in general?

@yoavweiss
Copy link
Contributor Author

BCD PR: mdn/browser-compat-data#24660

@hamishwillee
Copy link
Collaborator

@yoavweiss FYI back on Friday and will look at this then. Would be good to fix the whole doc in one go :-)

@chrisdavidmills chrisdavidmills removed their request for review October 9, 2024 09:53
@hamishwillee
Copy link
Collaborator

Thanks @yoavweiss .

I've read the explainers. I hope you can answer my inline questions. A few other things to confirm:

  1. A browsing context is an environment that hosts a document, such as a top level document, an iframe, a tab in a tab group, a popup.
  2. A browsing context group is a group of related browsing contexts that share some common "context".
    • Not entirely clear what they share, but I think it is things like browsing context - common history, at least at the point you open them? Cookies? Storage mechanisms?
  3. A top level window is a browsing context group.
    • The frames within the window are part of that group.
    • I think tabs of that window are also part of the group (right?)
    • I think the popups from it are part of a group by default but this is what you can affect using COOP (or opening with _noopener).
    • If you open a new tab is that also controlled by COOP?
  4. So COOP allows you to specify whether a browser opens a popup in a new browsing context or in the same context.
  5. The main benefit opening in a new browsing context is that the groups are process isolated (if supported by platform). This means they no longer share that context.
  6. Should we we have information about this case in https://developer.mozilla.org/en-US/docs/Web/API/Window/open
  7. I think we should mention here what is the difference between isolation and "not isolation". Specifically, we should mention or link in this doc Window.open(), Window/crossOriginIsolated and pull in the stuff about performance/shared data restriction from that last doc.

Thoughts?

Depending on your answer/answers to some of this we might merge the initial changes and then com back to this. I would like to see an example for this particular case though, because right now it is a little "abstract", and what you put in the explainer makes it a bit more real.

@yoavweiss
Copy link
Contributor Author

Again, I'd love @camillelamy to make sure I get it right..

  1. A browsing context is an environment that hosts a document, such as a top level document, an iframe, a tab in a tab group, a popup.

Yes

  • A browsing context group is a group of related browsing contexts that share some common "context".

    • Not entirely clear what they share, but I think it is things like browsing context - common history, at least at the point you open them? Cookies? Storage mechanisms?

That practically translates to top-level documents that need to maintain the ability to script each other ("retain references").

A top level window is a browsing context group.

  • The frames within the window are part of that group.
  • I think tabs of that window are also part of the group (right?)
  • I think the popups from it are part of a group by default but this is what you can affect using COOP (or opening with _noopener).
  • If you open a new tab is that also controlled by COOP?

I don't believe that's correct, but it's not obvious to me what you're referring to as "window" here (given "tabs").

A browsing context group is a set of top-level documents. ("tabs" % fenced frames, I think)
I'm not 100% sure, but it does seem like iframes are in the same group.

Popups can be in the same group, depending the results of the BCG check

I think new tabs create a new group.

  • So COOP allows you to specify whether a browser opens a popup in a new browsing context or in the same context.

Yes

5. The main benefit opening in a new browsing context is that the groups are process isolated (if supported by platform). This means they no longer share that context.

The benefits are that the different documents can be process isolated, as they don't have to be able to script each other. Cutting off the ability of them to be able to script each other also has security benefits beyond process isolation.

Potentially. It may be good to provide a high-level summary there and link here.

7. I think we should mention here what is the difference between isolation and "not isolation". Specifically, we should mention or link in this doc Window.open(), Window/crossOriginIsolated and pull in the stuff about performance/shared data restriction from that last doc.

I think that makes sense.

@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/xs [PR only] 0-5 LoC changed labels Oct 11, 2024
@hamishwillee hamishwillee requested a review from a team as a code owner October 14, 2024 05:39
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

@yoavweiss Thanks very much. Following your response on matching this all makes a lot more sense.

I have updated the directives and also added in tables, because I think they do make this a lot more clear: https://pr36232.content.dev.mdn.mozit.cloud/en-US/docs/Web/HTTP/Headers/Cross-Origin-Opener-Policy#description

I have approved and am happy to merge (will do a reread tomorrow). Leaving open for you to check if you wish though.

Copy link
Contributor Author

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

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

LGTM % a contradiction on same-origin-allow-popups

@hamishwillee
Copy link
Collaborator

Thanks @yoavweiss for your many reviews and patience. FWIW I think this is a much more usable document than what it replaces. It should also be much easier to update when/if there are further additions.

@hamishwillee hamishwillee merged commit 070ea0f into mdn:main Nov 12, 2024
8 checks passed
@yoavweiss
Copy link
Contributor Author

Thanks @yoavweiss for your many reviews and patience. FWIW I think this is a much more usable document than what it replaces. It should also be much easier to update when/if there are further additions.

Thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries Content:HTTP HTTP docs Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cover noopener-allow-popups COOP value
4 participants