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

F133 ServiceWorker in Workers #36726

Merged
merged 8 commits into from
Nov 22, 2024

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Nov 11, 2024

FF133 supports WorkerNavigator.serviceWorker, which now returns a ServiceWorkerContainer in worker scope.

This was supposed to update the docs appropriately for ^^^ but actually there isn't much to say about that. However I did notice some mistakes, and got thoroughly confused by some docs, so this is "tangentially related fixes" to those problems.

Related docs work can be tracked in #36532

@github-actions github-actions bot added Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed labels Nov 11, 2024
Copy link
Contributor

github-actions bot commented Nov 11, 2024

@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Nov 12, 2024
@hamishwillee hamishwillee marked this pull request as ready for review November 12, 2024 06:46
@hamishwillee hamishwillee requested a review from a team as a code owner November 12, 2024 06:46
@hamishwillee hamishwillee requested review from sideshowbarker and wbamberg and removed request for a team November 12, 2024 06:46
@hamishwillee
Copy link
Collaborator Author

@sideshowbarker @wbamberg I've put you both as reviewers. I hope to do more on this, but if I do I will be coming back to it later. I don't need this for the docs I have to do for FF release, I just went too far down the rabbit hole to waste the time before I realised that.

If you don't get to this I'll build on it later.

@@ -107,7 +107,8 @@ This could be for the following reasons:

After your service worker is registered, the browser will attempt to install then activate the service worker for your page/site.

The `install` event is fired when an installation is successfully completed. The `install` event is generally used to populate your browser's offline caching capabilities with the assets you need to run your app offline. To do this, we use Service Worker's storage API — [`cache`](/en-US/docs/Web/API/Cache) — a global object on the service worker that allows us to store assets delivered by responses, and keyed by their requests. This API works in a similar way to the browser's standard cache, but it is specific to your domain. The contents of the cache are kept until you clear them.
The `install` event is the first event that is fired after registration is successfully completed and the service worker executes.
It is emitted only once for every service worker, and is generally used to populate your browser's offline caching capabilities with the assets you need to run your app offline. To do this, we use Service Worker's storage API — [`cache`](/en-US/docs/Web/API/Cache) — a global object on the service worker that allows us to store assets delivered by responses, and keyed by their requests. This API works in a similar way to the browser's standard cache, but it is specific to your domain. The contents of the cache are kept until you clear them.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"It is emitted only once for every service worker" - is this quite right? Isn't it also emitted when a service worker is updated? (e.g. https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers#updating_your_service_worker)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

files/en-us/web/api/serviceworker/index.md Outdated Show resolved Hide resolved
The exception is also raised if the `scriptURL` or `scope URL` path contains the case-insensitive ASCII "%2f" (`*`) or "%5c" (`=`)

- `SecurityError` {{domxref("DOMException")}}
- : The `scriptURL` is not a potentially trustworthy origin, such as `localhost` or an `https` URL.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how we throw one exception if the scheme is http or https (see above) when actually only https is allowed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is odd. Reading through the spec the checks are first on http and https which gives you a typeerror, Then later on there is a check on it being a potentially trustworthy origin.

So to answer your question, localhost considered a potentially trustworthy URL even if it is the normal http localhost.
So http is allowed in localhost.

Does that make sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, I don't talk about these requirements up in scriptURL because they are implied by the secure context header.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that make sense?

Yes, it does (I saw what you had here looked correct per spec, I just thought the spec was weird. But what you say makes sense).

@hamishwillee
Copy link
Collaborator Author

Thanks @wbamberg @sideshowbarker . Suggestions/feedback integrated. One open question.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thank you Hamish! I want to say, we should have a standalone article on SW lifecycle, but I doubt if we could do better than https://web.dev/articles/service-worker-lifecycle.

@wbamberg wbamberg merged commit 5d29bef into mdn:main Nov 22, 2024
8 checks passed
@hamishwillee hamishwillee deleted the ff133serviceworkercontainer_inworkers branch November 22, 2024 03:55
@hamishwillee
Copy link
Collaborator Author

You're welcome. I hope to extend some of this information to the registration class too in the next few days.

I want to say, we should have a standalone article on SW lifecycle, but I doubt if we could do better than https://web.dev/articles/service-worker-lifecycle.

Yeah, that's a good doc. It's a pity we can't copy it.

Reading https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers did not give me a great view on the questions I had around how this all works. It is hard to fairly compare two docs that approach a similar problem in such different way and level, but I do feel that I understood what service workers were trying to achieve better and more quickly with the web.dev doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants