core(service-worker): check that test page is in SW scope #6609
Conversation
brendankenny
requested review from
patrickhulce and
paulirish
as code owners
| const matchingSW = versions.filter(v => v.status === 'activated') | ||
| .find(v => new URL(v.scriptURL).origin === origin); | ||
| // Ensure page is included in a SW's scope. | ||
| // See https://w3c.github.io/ServiceWorker/v1/#scope-match-algorithm |
There was a problem hiding this comment.
I was really hoping this would just use Use .startsWith 😆
| const swOpts = [{ | ||
| status: 'activated', | ||
| scriptURL: 'https://example.com/serviceworker/sw.js', | ||
| scopeURL: 'https://example.com/', |
There was a problem hiding this comment.
this can happen? for some reason I thought sw could only control it's own directory
EDIT: MDN still thinks so too 🤔
There is frequent confusion surrounding the meaning and use of scope. Since a service worker can't have a scope broader than its own location, only use the scope option when you need a scope that is narrower than the default.
There was a problem hiding this comment.
No, good point. I went too wild with the possible combinations of states :)
There was a problem hiding this comment.
OK, actually I was wrong again. The service worker's directory is the "max scope" and it can only be narrowed by serviceWorker.register(), however if you serve the SW with a Service-Worker-Allowed header, you can change the max scope to something higher up. Google Maps does this, for instance.
There was a problem hiding this comment.
added a comment to explain
There was a problem hiding this comment.
ah OK, they should update some of those docs :)
https://www.w3.org/TR/service-workers-1/#service-worker-script-response was a pretty good example too if ya want to throw that example in there
Yeah, stepping stones, I think, to help figure out why the SW isn't working (it shouldn't show passing when the offline check is failing specifically because the SW has the wrong scope). I imagine it will only fail when first starting to use a SW or someone makes a typo :) |
brendankenny
force-pushed
the
swscope
branch
from
3ff95b8
to
aa06403
Compare
| // artifacts.URL.finalUrl so audit accounts for any redirects. | ||
| const versions = artifacts.ServiceWorker.versions; | ||
| const url = artifacts.URL.finalUrl; | ||
| const matchingSWVersions = versions.filter(v => v.status === 'activated') |
There was a problem hiding this comment.
do we want to consider 'activating' or even 'installed'/'installing' ?
I can't really tell if including those cases could give a false positive.
There was a problem hiding this comment.
I'm not sure. The gatherer runs in beforePass of the offline pass...if it's just installing I don't think it'll be ready to serve assets for that pass. I'm not even sure if this is a possible state...if you're installing and a navigation to about:blank and back occurs, does it keep installing?
There was a problem hiding this comment.
k sg. since we're in beforePass of offline, this seems entirely reasonable.
part of #6395
add an additional check to the
service-workeraudit that the test page is within the scope of a SW.I'll follow up with a second PR that tests that the
start_url(if it exists) is within scope. Right now this is just pass/failrawValue, but we'll probably need an auditexplanationwith the second PR.