Remove fragment identifiers from script url and scope url #1251
Conversation
The spec and the implementations did not agree on the behavior: the spec didn't remove fragment identifiers and had ServiceWorker.scriptURL and ServiceWorkerRegistration.scope return urls without stripping them. Chrome, Firefox, and Edge don't return fragments. This changes the spec to remove the fragment indentifiers from the script url and scope url given to register() method before storing them, in order to match the implementations. Fixes #1249.
|
FYI, I have just landed a patch in WebKit to strip the fragment identifier in WebKit, to align with Firefox and Chrome. |
|
FWIW, it seems we added the fragment stripping here: https://bugzilla.mozilla.org/show_bug.cgi?id=1130688 But I don't see anything in the related spec change that calls that out: Its possible we were trying to match a WPT test or something. |
|
Or maybe we removed it because we also remove the fragment from the scope and that spec change was about trying to compare scriptURL against the scope for validity. |
There was a problem hiding this comment.
https://pr-preview.s3.amazonaws.com/w3c/ServiceWorker/1251/90c04cd...c3455b0.html#clear-registration-algorithm step 5 – we don't need the exclude fragment flag anymore right?
https://pr-preview.s3.amazonaws.com/w3c/ServiceWorker/1251/90c04cd...c3455b0.html#get-registration-algorithm step 3 – we don't need the exclude fragment flag, but maybe this one is less obvious. Perhaps move it to a note?
- getRegistration(): Set the given url's fragment to null. - Set Registration: Revert to set the exclude fragment flag. - Clear Registration: Change to not set the exclude fragment flag. - Add notes. Set Registration and Get Registration take a url as an argument. So, it'd be safer to set the exclude fragment flag.
Likely!
Yes, I changed it to not set the flag.
Having thought about it, I think Get Registration and Set Registration should set the flag as they take a url as an argument. Currently they are invoked only from SW internal algorithms, but if they were invoked from outside, that flag would be needed. I intentionally left Match Service Worker Registration step 2 to not set the flag as the given url's fragment doesn't have an effect on matching the scope. |
The spec and the implementations did not agree on the behavior: the spec
didn't remove fragment identifiers and had ServiceWorker.scriptURL and
ServiceWorkerRegistration.scope return urls without stripping them.
Chrome, Firefox, and Edge don't return fragments.
This changes the spec to remove the fragment indentifiers from the
script url and scope url given to register() method before storing them,
in order to match the implementations.
Fixes #1249.
Preview | Diff