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

Fix non-fetch-scheme-params-initiator-origin assignment #9532

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

domfarolino
Copy link
Member

@domfarolino domfarolino commented Jul 18, 2023

This PR fixes #9517 by setting the non-fetch-scheme-params-initiator-origin member to responseOrigin in #create-navigation-params-by-fetching. This allows #hand-off-to-external-software to show the origin of the last redirect in the chain that led to a non-fetch-scheme, which is what all browsers that display an origin-titled prompt do (Chrome & Firefox). Safari just seems to ask for general permission without mentioning which site is initiating, which is allowed per the spec.

It took me a minute to confirm that responseOrigin was indeed the right origin to grab as a described above (I was initially worried that it included the response's location URL, which has no sensible "origin"). I always have to think that through when reading this algorithm, so this PR also adds a note to describe that responseOrigin has nothing to do with the "location URL".

It doesn't seem possible to test this with WPTs, which is also the case when we refactored much of the non-fetch-scheme params stuff in the navigation & session history rewrite #6315. Nevertheless this PR fixes a glaring issue in the non-fetch-scheme navigation path.


/browsing-the-web.html ( diff )

data-x="she-document-state">document state</span>'s <span
data-x="document-state-initiator-origin">initiator origin</span>.</p>

<p class="note">If <var>response</var> is a redirect, then <var>response</var>'s <span
Copy link
Member Author

Choose a reason for hiding this comment

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

See the OP for this. If this note is too heavy on the documentation let me know and I'll remove it, but since I have to think this through every time I read this algorithm, I figured it could be helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Since this is the only part of the platform that manually handles redirects, I think it's very reasonable to include this sort of thing.

@domenic domenic merged commit 238086f into main Jul 19, 2023
1 check passed
@domenic domenic deleted the domfarolino/non-fetch-scheme-params-initiator-origin branch July 19, 2023 07:17
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.

Non-fetch scheme navigation params initiator origin isn't always set correctly
2 participants