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

Editorial: use origin-based "same site" definition #965

Merged
merged 4 commits into from
Nov 20, 2019
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Nov 11, 2019

Follows whatwg/html#5076.

/cc @mikewest

I'm unsure whether this should be "same site" or "schemelessly same site". I guess for parity with the previous it should be "schemelessly same site"? But that seems like a pretty bad mismatch with the web-dev-visible header name... what's going on?


Preview | Diff

@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label Nov 11, 2019
@mikewest
Copy link
Member

I think the goal will be to change our collective behavior such that we take the scheme into account for mechanisms that work through today's "same site" concept. So yes, today it's a mismatch. Tomorrow, it ideally won't be.

Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

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

👍

@annevk
Copy link
Member

annevk commented Nov 12, 2019

The second bullet point which you did not touch is important here and follows from a suggestion by @youennf in #687 (comment), also supported by @arturjanc and @johnwilander at the time.

Soon those mixed content images will no longer be requested over HTTP though so I think we can indeed simplify this, but that means two things for this PR:

  1. The second bullet point needs to go and perhaps that will let us remove the list altogether.
  2. It's no longer editorial.

@domenic
Copy link
Member Author

domenic commented Nov 18, 2019

OK, I think I changed this as people were suggesting. However I'm a bit unsure whether or how we should tie this up with the updates for whatwg/url#448. Any guidance would be appreciated.

To be clear, I was mostly hoping to make editorial changes with this effort, and would rather not sign up for e.g. updating web platform tests for CORP, if we're making normative changes to its behavior.

@annevk
Copy link
Member

annevk commented Nov 19, 2019

In that case you should replace same site with schemelessly same site in the existing text and we can file a follow-up to consider making this stricter (which would then end up with the simplified text).

@domenic
Copy link
Member Author

domenic commented Nov 19, 2019

Ok, and keep the second bullet point? In that case I think it's just a revert of my second commit, right?

@annevk
Copy link
Member

annevk commented Nov 19, 2019

Well, and s/same site/schemelessly same site/ as that was what same site used to do.

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 19, 2019
@domenic
Copy link
Member Author

domenic commented Nov 19, 2019

OK cool, this should be ready to merge when the Bikeshed link database updates. Commit message:

Editorial: use origin-based "schemelessly same site"

Follows https://github.com/whatwg/html/pull/5076. See
https://github.com/whatwg/fetch/issues/969 for a potential normative
follow-up to change this to use "same site"

@domenic domenic merged commit 493c021 into master Nov 20, 2019
@domenic domenic deleted the domenic/site branch November 20, 2019 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants