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

Scope should remove the query and fragment at parse time #793

Closed
mgiuca opened this issue Sep 16, 2019 · 3 comments · Fixed by #961
Closed

Scope should remove the query and fragment at parse time #793

mgiuca opened this issue Sep 16, 2019 · 3 comments · Fixed by #961
Labels
scope member Related to scope member

Comments

@mgiuca
Copy link
Collaborator

mgiuca commented Sep 16, 2019

Since our within scope algorithm ignores the query and fragment part of the scope, I feel like we should strip those out in the steps for processing the scope.

Thus if your scope is "/foo/?bar", we would canonically say that the scope is "/foo/", reflecting the fact that the query string is irrelevant.

This change would not actually have any direct (specified) user-visible changes, since the only place in the manifest spec that scope is used is the "within scope" algorithm (which ignores query and fragment). However, it might be exposed to the user, for example, through developer tools that displays the canonical parse of the manifest, in which case it would be best to strip the query and fragment for clarity.

@aarongustafson
Copy link
Collaborator

Makes sense to me.

Just to head off potential questions: this applies to scope only, not start_url (which we have another discussion about with regard to query strings and privacy).

@mgiuca
Copy link
Collaborator Author

mgiuca commented Sep 16, 2019

Yes, only scope.

Also note that there is a proposal to extend scope syntax with query matching --- this change would only apply to legacy scope syntax which doesn't support query matching in manifest.

@fallaciousreasoning fallaciousreasoning added the scope member Related to scope member label Sep 20, 2019
@fallaciousreasoning
Copy link
Collaborator

fallaciousreasoning commented Sep 20, 2019

Note: I'm pretty sure ServiceWorkers don't do this, so we'll be diverging a bit.

^ Though I guess we've already diverged, because we ignore it.

I like this though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope member Related to scope member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants