Skip to content

Make Service Worker registration work properly (don't use outdated steps) - #792

Merged
marcoscaceres merged 4 commits into
w3c:gh-pagesfrom
mgiuca:service-worker-start-register
Nov 12, 2019
Merged

Make Service Worker registration work properly (don't use outdated steps)#792
marcoscaceres merged 4 commits into
w3c:gh-pagesfrom
mgiuca:service-worker-start-register

Conversation

@mgiuca

@mgiuca mgiuca commented Sep 13, 2019

Copy link
Copy Markdown
Collaborator

This change (choose one):

  • Breaks existing normative behavior (please add label "breaking")
  • Adds new normative requirements
  • Adds new normative recommendations or optional items
  • Makes only editorial changes (only changes informative sections, or
    changes normative sections without changing behavior)
  • Is a "chore" (metadata, formatting, fixing warnings, etc).

Commit message:

Make Service Worker registration work properly (don't use outdated steps).

Closes #789.


Preview | Diff

Comment thread index.html
A {{Document}} may either be <dfn>installable</dfn> or not. The initial
state of a document is not <a>installable</a>.
</p>
<p>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this PR have unrelated changes? It seems so

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, yes, this is because GitHub doesn't respect upstream branches in Git. This PR is dependent on #790 which hasn't landed yet, and I don't think there's any way for GitHub to show just the relevant changes.

Please look at this delta.

@mgiuca

mgiuca commented Sep 13, 2019

Copy link
Copy Markdown
Collaborator Author

@marcoscaceres While I'm on a roll with Manifest...

Edit: Please look at this delta.

  1. I need a bit of help with ReSpec. (General ReSpec comments appreciated; I've been trying out the new (?) |var|.{{Method}} syntax.) I tried to add a reference to RegistrationOptions.type using "for="RegistrationOptions"" (the "for" clause working well for all the other methods and members). But I get a conflict with the other definition of ECMAScript's Type() function. I thought "for" was supposed to disambiguate it by saying "this type is for RegistrationOptions only" but it still conflicts.
  2. This is a bit nasty but it's in the current text too: (the old text reads: "the state of the settled promise determines whether the installation succeeded or not") --- the fact that if service worker registration fails, the whole installation is aborted, even though service worker registration is optional. So some user agents will not bother to register a (failing) SW, thus successfully installing, and others will try to register the SW and fail the install. Is that "okay" (essentially making a best effort to check, but you don't have to check). Or should we succeed the install even if SW registration fails, for consistency across user agents?

@mgiuca mgiuca mentioned this pull request Sep 20, 2019
5 tasks
@mgiuca
mgiuca force-pushed the service-worker-start-register branch from 5a56e10 to 7a551e3 Compare September 26, 2019 06:51
@mgiuca

mgiuca commented Sep 26, 2019

Copy link
Copy Markdown
Collaborator Author

The upstream PR has now merged, so this PR's delta is correct.

@marcoscaceres marcoscaceres left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved with caveat the the service worker stuff will be deleted right after (as decided in #800).

@kenchris

Copy link
Copy Markdown
Collaborator

Well Jeffrey (talked at TPAC) says to please not do that as he is going to use that innWeb Packaging which uses web app manifest

@kenchris

Copy link
Copy Markdown
Collaborator

@jyasskin could you comment?

@marcoscaceres

Copy link
Copy Markdown
Member

Sure, but the multi-implementer problem remains. Not sure at this point we want to support this in Gecko, but need to check if WebKit supports it.

@jyasskin

jyasskin commented Oct 2, 2019

Copy link
Copy Markdown
Member

We might use the serviceworker field to have navigation to a web package automatically install its service worker before the start page loads, but I think we can re-add the field if/when we do that. It won't hurt us to remove it until then.

@marcoscaceres

Copy link
Copy Markdown
Member

Ok, let's proceed with the plan to remove it... we can always bring it back.

@marcoscaceres
marcoscaceres merged commit 6708bc8 into w3c:gh-pages Nov 12, 2019
@mgiuca

mgiuca commented Nov 14, 2019

Copy link
Copy Markdown
Collaborator Author

Oops, thanks for closing this out.

christianliebel pushed a commit to christianliebel/manifest that referenced this pull request May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Service worker installation uses an algorithm (Start Register) that doesn't exist in SW spec

4 participants