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.

Sign up for GitHub

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

Jump to bottom

Only serve offline/error page if Offline Browsing is enabled; conditionally install service worker #436

Open
westonruter opened this issue Mar 4, 2021 · 3 comments · May be fixed by #727
Open

Only serve offline/error page if Offline Browsing is enabled; conditionally install service worker #436

westonruter opened this issue Mar 4, 2021 · 3 comments · May be fixed by #727
Labels
service-workers

Comments

@westonruter
Copy link
Collaborator

As mentioned in #344 (comment):

I wonder if this should also control whether we serve the Offline page and the Server Error page.

This was not implemented in #365 but after thinking about it more, it seems the right way to go. Furthermore, the service worker should only ever be installed if there is any SW logic registered to run in it. So if the offline/error page is included with offline browsing, and offline browsing is not enabled, then there would be no SW logic registered and the SW should be skipped from being installed. As soon as that checkbox is enabled or if a plugin registers a script to run in the service worker, then the SW would then be installed automatically.

There probably needs to be a new conditional function added to check whether the SW is being installed.

By not installing the service worker by default, we'll avoid situations where the SW is responding to requests unexpectedly, which can cause confusion. It will also reduce extraneous SW installation across the web, reducing the service workers that are installed but aren't doing anything much of value.
@westonruter westonruter added this to the 0.7 milestone Mar 4, 2021
@westonruter westonruter mentioned this issue Feb 28, 2022
Closed
@rutviksavsani
Copy link
Contributor

rutviksavsani commented Mar 4, 2022 •
edited
Loading

@westonruter I did have a few queries regarding this task, It would be great if you can answer them.

  1. Do we provide users an option to allow displaying offline/error pages similar to offline browsing?
  2. Can you please elaborate on the statement

    There probably needs to be a new conditional function added to check whether the SW is being installed.

  3. What about the default scripts that we are loading in the service worker, In what way should they be taken care of?

@westonruter
Copy link
Collaborator Author

  1. Do we provide users an option to allow displaying offline/error pages similar to offline browsing?

This would essentially mean that the current Offline Browsing checkbox in Reading settings would be the opt-in to not only cache page navigations but also enable the offline page.

There probably needs to be a new conditional function added to check whether the SW is being installed.

Basically there would be a helper function to see if there have been any registered SW scripts for a given scope.

What about the default scripts that we are loading in the service worker, In what way should they be taken care of?

We'd ignore the default scripts. So if only the default scripts are present, we'd skip installing the service worker since it wouldn't be doing anything.

@rutviksavsani rutviksavsani linked a pull request Mar 10, 2022 that will close this issue
Open
@westonruter
Copy link
Collaborator Author

Since this is a more substantial change that may have unexpected behavior for users who previously have been passing the PWA check without enabling offline browsing, I'm going to punt this to 0.8 so we can think about it some more.

@westonruter westonruter modified the milestones: 0.7, 0.8 Apr 15, 2022
@westonruter westonruter modified the milestones: 0.8, 0.9 Jan 24, 2024
@westonruter westonruter modified the milestones: 0.8.1, 0.9 Apr 2, 2024
@westonruter westonruter removed this from the 0.8.2 milestone Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
service-workers
Projects
None yet
Milestone
No milestone
2 participants
@westonruter @rutviksavsani