-
Notifications
You must be signed in to change notification settings - Fork 312
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
Default scope to register() #595
Comments
It should be URL('./', scriptURL).href. I can imagine service worker registration being part of the script that's included in most pages. If "./" was the default scope users would end up with many ServiceWorkers as they navigate around the site. |
Got it, thanks! |
One thing to clarify is whether we'd want to use script url (rather than the page url) as the base url to parse all the given scope urls (i.e. not only the default scope url). I'd assume so because otherwise we'll have to special case the default value "./" which might confuse authors. For example, we'd want the following output without the special casing the default value, right? // example.com/foo/page.html
var sw = navigator.serviceWorker;
sw.register("bar/sw.js"); // scope == "/foo/bar/"
sw.register("bar/sw.js", { scope: "./" }); // scope == "/foo/bar/" (not "/foo/")
sw.register("bar/sw.js", { scope: "baz/" }); // scope == "/foo/bar/baz/" (not "/foo/baz/") |
But if using the page url as the base url seems rather natural in general, I can define something like a max scope initialized to the value of URL('./', scriptURL).href and use this in prose rather than setting the default value in IDL. |
I think it's ok for the default url and base url to be unrelated. ie., the following seems acceptable to me: // example.com/foo/page.html Anyway, this should be decided as soon as possible, as changing it later will likely break sites. |
Agreed. The default url for a scope seems more like just a constraint than a general rule for parsing scope urls. |
The default scope text has been sorted out, and the security condition for checking a scope against the location of the script url has been added: f23c1d4. |
Agreed with @mattto's example, spec look good too! |
Shall this bug be closed? Spec changes tracking note:
|
Yes, it's done! Closing. |
The default scope was '/' which will usually result in failure due to the path restriction. This patch changes the scope to URL('./', scriptURL).href as per w3c/ServiceWorker#595 (comment) BUG=445154 Review URL: https://codereview.chromium.org/822883003 git-svn-id: svn://svn.chromium.org/blink/trunk@187812 bbb929c8-8fbe-4397-9dbb-9b2b20218538
Currently the default scope is "/" which will likely result in failure due to the path restriction. There was some suggestions to revise it around #468 (comment) but it seems not decided yet.
Chrome is happy making it something like "./". I'm not actually sure the difference between './' and new URL('./', scriptURL).href
The text was updated successfully, but these errors were encountered: