-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Globalutils and self.origin #951
Conversation
@@ -87428,6 +87428,8 @@ interface <dfn>DocumentAndElementEventHandlers</dfn> { | |||
|
|||
[NoInterfaceObject, Exposed=(Window,Worker)] | |||
interface <dfn>GlobalUtils</dfn> { | |||
[Replaceable] readonly attribute USVString <span data-x="dom-origin">origin</span>; |
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.
Why Replaceable?
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.
I figured new short properties on the global object need to be replaceable to ease compatibility concerns. If @bzbarsky thinks otherwise I'd be happy to drop it. bz was also who I had in mind as implementer interest from the Gecko side (though not necessarily the person implementing).
Can we get some implementer commitments for this new feature? |
<p>The <code data-x="dom-windowbase64-atob">atob()</code> and <code | ||
data-x="dom-windowbase64-btoa">btoa()</code> methods allow authors to transform content to and from | ||
the base64 encoding.</p> | ||
<h3 id="globalutils-mixin">The <code>GlobalUtils</code> mixin</h3> |
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.
I think WindowWorkerUtils is probably better, given Worklets...
Looks ok to me for the "origin" part, as long as we actualy define the origin of every global. I didn't really look at the other pieces. |
Thanks. |
Let's move to WindowOrWorkerUtils and then we can merge the first commit. The second commit needs more implementer interest though. @mikewest, @jeisinger, you guys do origin stuff in Blink; any interest in adding |
|
||
<!-- v2: actual binary support --> | ||
[NoInterfaceObject, Exposed=(Window,Worker)] | ||
interface <dfn>GlobalUtils</dfn> { |
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.
I don't like WindowOrWorkerUtils. I suggest WindowOrWorkerGlobalScope. That's more consistent with DOM mixins. Waiting with fixing this though until we heard back from Googlers, not entirely sure how to merge a single commit.
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.
WindowOrWorkerGlobalScope works for me. I can merge a single commit (or walk you through it) if you want.
Just to clarify, the attribute should return the origin, not the effective origin? |
@jeisinger correct (also, note I merged effective script origin into origin with #925 so the specification no longer has that concept). |
I don't have any objection to exposing the global object's origin. Would we drop |
k, should be easy to implement |
@mikewest see https://bugzilla.mozilla.org/show_bug.cgi?id=931884#c11 where I asked about that. In reply @bzbarsky suggested to have both. (And to try to get folks to not use |
Renamed the mixin to |
So we'll have three origins, right? |
I think |
So interestingly, So maybe we should remove |
Regardless I think we should add the example you showed to make it clear how |
Yeah, I would be okay with removing I will add an example contrasting |
We actually have four things: I guess the behavior of bareword |
Bareword |
If you want to view it that way, yes. |
<p>The <code data-x="dom-windowbase64-atob">atob()</code> and <code | ||
data-x="dom-windowbase64-btoa">btoa()</code> methods allow authors to transform content to and from | ||
the base64 encoding.</p> | ||
<h3 id="windoworworkerglobalscope-mixin">The <code>WindowOrWorkerGlobalScope</code> mixin</h3> |
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.
This probably should get an intro paragraph explaining what it is and that other specs can extend it. Just a section with an IDL block kind of sucks.
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.
Done, also added an example.
This makes the following editorial changes: * Groups base64, timer, and ImageBitmap properties on a single mixin. * Removes overloading of timer methods in favor of a union type It is expected that other standards will make use of this mixin, e.g., Fetch will use it to define fetch().
OK cool, this looks good to me; merging. Still unsure about whether [Replaceable] is a great idea, but I guess it's safer that way. |
See commit messages for details.