core(service-worker): check that start_url is within SW's scope #6678
Conversation
brendankenny
requested review from
exterkamp,
patrickhulce and
paulirish
as code owners
| }; | ||
| } | ||
|
|
||
| const startUrlFailure = ServiceWorker.checkStartUrl(artifacts.Manifest, controllingScopeUrl); |
There was a problem hiding this comment.
wouldn't we just want to check that a service worker controls the start URL? i.e. if the one controlling this page is more specific but the start URL is the homepage and so it's controlled by a root service worker for example.
There was a problem hiding this comment.
wouldn't we just want to check that a service worker controls the start URL?
Yeah, we may need to think about this a bit. I originally had that check, and I don't think there's any technical problem with it, but then was thinking of the issues if you're testing a page and the declared web app manifest creating the PWA containing that page had a start_url that was controlled by a different SW than the page...so I guess the start_url is part of a different PWA than the PWA the test page is part of?
So then that seems to violate testing the page as part of a PWA. But also I don't know. But also this should only be possible when running with --disable-storage-reset anyways, so it's fine to be tougher or not tougher on them :)
There was a problem hiding this comment.
As an example: google.com/maps has a service worker controlling everything under the google.com/maps scope. So if you go to google.com/maps, you're served offline from that SW.
However, if they accidentally put a start_url of google.com in their manifest (and google.com has a SW), trying to load the maps PWA (e.g. from the homescreen) would load a completely different PWA than the one that you saved.
Again, the likely Lighthouse user experience if someone ever accidentally does this is that storage was cleared and the start_url just doesn't work offline, but in the off chance they're running without clearing, I think it's better to fail on that case.
There was a problem hiding this comment.
Ah, OK I buy that! SGTM
| const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl)); | ||
| assert.equal(output.rawValue, false); | ||
| const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); | ||
| expect(output).toMatchObject({ |
| const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl, manifest)); | ||
| expect(output).toMatchObject({ | ||
| rawValue: false, | ||
| explanation: expect.stringMatching(new RegExp(`${finalUrl}.*not in scope`)), |
There was a problem hiding this comment.
well TIL! stringMatching and arrayMatching are dope
| }); | ||
| }); | ||
|
|
||
| it('fails when SW that controls start_url is different than SW that controls page', () => { |
There was a problem hiding this comment.
oh it seems like it was intentional then, why is that?
part of #6395
Adds a check to the
service-workeraudit to verify that thestart_urlis within the SW's scope.This joins the #6609 check that the page itself is in scope. As with that check, this is mostly useful when first setting up the SW or someone accidently alters the scope somehow. The explanation for this audit will help with debugging other failures (e.g. offline).