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
Fix #2: Clarify there is one battery promise per Document #3
Conversation
Shouldn't we include workers too? |
@mounirlamouri I'd consider expose to workers a v2 feature (feel free to open a separate issue for that). |
lgtm then |
(I don't have write access so I can't merge.) |
This isn't quite sufficient. When creating the BatteryManager object, you need to be clear what realm it's being created in. |
For example, if I run code in window3 that does |
@mounirlamouri You have write access now (sorry, my bad). @domenic To be consistent with the rest of the platform, with which realm you'd suggest we associated the |
I'm not sure. @bzbarsky may have an opinion. To rephrase the question, given code in For that matter, which of the three realms is the Promise created in? :-/ |
Ah, I found the thread I was searching for where @bzbarsky expressed an opinion previously: w3ctag/promises-guide#46 (comment) So, now that I remember his reasoning, After a meeting I have to dash to I'll get back with some proposed spec text. |
Suggested:
|
It's not clear to me that Document and Navigator are equivalent here. Aren't Navigator objects per-Window, not per-Document? Apart from that, I agree that using the Navigator object's realm for the Promise and the BatteryManager seems like the right thing. |
Would anyone object us merging this PR and use that for the Rec Track advancement, while we in parallel incubate @domenic's Realm patch #3 (comment) in the Editor's Draft and after baked in release a Proposed (Edited) Recommendation that integrates these further clarifications? I believe that'd minimize the process-related back-and-forths and allow us to make forward progress on all the fronts. |
No, this PR is not sufficient for REC. |
Ok, I was not clear whether @bzbarsky was actually suggesting further changes to @domenic's patch #3 (comment). If the patch is good as is, I'll update this PR. |
Boris is right that Navigator and Document are not equivalent. In particular, given the navigation from the original That said, I think using Navigator is better. In those situations, I would expect the same promise to be returned for about:blank navigation, and a new promise to be returned after document.open(). But we should ask @mounirlamouri what he thinks as an implementer since he says in Blink uses Document. If @mounirlamouri prefers Document, then amend my above to replace "this Navigator object's battery promise" with "this Navigator object's relevant settings object's global object's newest |
- Add Realms [ECMASCRIPT] reference to Terminology - Editorial: use a numbered list for the getBattery() steps
@domenic @mounirlamouri Updated the PR, please take a look, thanks! |
@@ -149,12 +150,15 @@ | |||
<code><dfn><a href= | |||
"https://www.w3.org/TR/html5/dom.html#document">Document</a></dfn></code> |
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.
Document appears to be unused in this file now
LGTM with nits. But definitely needs @mounirlamouri's input given that it's different than Blink's implementation. |
@domenic Much thanks for the review. I fixed your nits with f7c9a02. @mounirlamouri Please let us know if you're happy about the prose in this PR or whether to go the other way described in #3 (comment) |
lgtm. Actually, Blink has one |
Thanks! Ping @dontcallmedom re possible process implications. |
thanks; it will be easier to assess the process implications once we have an updated test suite and clarity as to whether the spec change does indeed match the existing browser behavior |
…avigator object Corresponding spec changes are at w3c/battery#3 and the original issue is at w3c/battery#2
…avigator object Corresponding spec changes are at w3c/battery#3 and the original issue is at w3c/battery#2
…avigator object (#2962) Corresponding spec changes are at w3c/battery#3 and the original issue is at w3c/battery#2
…avigator object (web-platform-tests#2962) Corresponding spec changes are at w3c/battery#3 and the original issue is at w3c/battery#2
…avigator object (web-platform-tests#2962) Corresponding spec changes are at w3c/battery#3 and the original issue is at w3c/battery#2
Review appreciated from @domenic @mounirlamouri et al.