-
Notifications
You must be signed in to change notification settings - Fork 1
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
Address comments in the main PR. #12
Conversation
@sisidovski @domenic Can I ask you to take a look? |
@@ -1594,10 +1595,11 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/ | |||
1. Let |routerRules| be a copy of |serviceWorker|'s [=list of router rules=]. | |||
1. If |rules| is a {{RouterRule}} dictionary, set |rules| to « |rules| ». | |||
1. For each |rule| of |rules|: | |||
1. If running [=Verify Router Condition=] algorithm with |rule|["{{RouterRule/condition}}"] and |serviceWorker| returns false, [=throw=] a {{TypeError}}. | |||
1. If running [=Verify Router Condition=] algorithm with |rule|["{{RouterRule/condition}}"] and |serviceWorker| returns false, return [=a promise rejected with=] a {{TypeError}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(as an aside) these changes aren't technically needed since webidl will convert any thrown exceptions into rejected promises anyway, but having them does make it clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure.
docs/index.bs
Outdated
@@ -3127,6 +3129,7 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/ | |||
|
|||
1. [=Abort a running script|Abort the script=] currently running in |serviceWorker|. | |||
1. Set |serviceWorker|'s [=start status=] to null. | |||
1. Unset |serviceWorker|'s [=ServiceWorkerGlobalScope is ready=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also address this comment?
https://github.com/w3c/ServiceWorker/pull/1701/files#r1484868955
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partially addressed the comment by adding " flag". However, I believe the main topic of the comment is renaming.
I am considering to call it "ServiceWorkerGlobalScope CSP ready flag"
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing substantial from me on this PR
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
- returns true. - renamed to add "flag", but I still need to find a better name.
Although not all reviewers approved this PR, I am going to merge this so that people can see the full PR in w3c#1701. |
There is ongoing PR review in w3c#1701.
I have addressed some of them here.
Until #10 is merged, I plan to work here to avoid #10 review and update exhausted.
Preview | Diff