Spec for window.fence.notifyEvent and onfencedtreeclick #156
Conversation
spec.bs
Outdated
| attributes to `true`. When running the | ||
| <a href="https://dom.spec.whatwg.org/#inner-event-creation-steps">inner event creation steps</a>, | ||
| set the <var ignore=''>time</var> to a user-agent-defined value that is consistent across all | ||
| invocations of this method. |
There was a problem hiding this comment.
Is this part special in some way, or unique to this kind of event? It seems a little funny.
There was a problem hiding this comment.
Yeah, the idea was to not provide specific timestamps from notifyEvent() to the embedder. Probably not strictly necessary given that the embedder can grab their own close-enough timestamp, but follows the principle of passing as little data to the embedder as possible.
There was a problem hiding this comment.
Hmm, well the event should actually be created inside the parent asynchronously as part of the task queued by the child anyways, so I'm not sure how useful that is at all since the parent is ultimately in charge of firing the event. There is no information in the event that the parent doesn't already have access to.
There was a problem hiding this comment.
Based on conversation in the TPAC breakout, I still think this censored timestamp field is good to keep (even if the parent knows all the information, it's still subject to the time taken between when the fenced frame sends notifyEvent and when the browser actually pops the event listener off of the event loop).
There was a problem hiding this comment.
So what should the value actually be? Should we just make it always null or something? What does it mean for it to be consistent across all invocations of this method? Certainly a proper "time stamp" can't behave like that, right? (Hence why I ask if we should make it "null")
There was a problem hiding this comment.
When writing the Chromium implementation, I spent a while thinking about this. Chromium has a TODO to remove remove the concept of "null" time. As a result, I used the Unix epoch instead, since that's a well-known fixed value. I figured given that this timestamp is intentionally a placeholder, its exact value isn't that important.
If we'd like to pick a specific value instead of leaving it up to the user agent, we could just use 0, or the Unix epoch, given that it's defined in the same spec as DomHighResTimeStamp.
By "consistent across all invocations of this method" I meant: any time a "fencedtreeclick" event is fired, regardless of what the actual current time is, all event objects will have the same timestamp.
Thinking about it more, I'm leaning toward just using the Unix epoch in this spec directly, to make things less ambiguous.
spec.bs
Outdated
|
|
||
| 1. [=Consume user activation=] for [=this=]'s [=relevant global object=]. | ||
|
|
||
| 1. [=Fire an event=] named <code>[=fencedtreeclick=]</code> at |navigable|'s |
There was a problem hiding this comment.
You can't fire this synchronously from here. I assume the implementation doesn't either, right (I'm pretty sure we asyncly go through the browser process and mojo and so on)? It should be asynchronous probably posting a task to the unfenced parent's Window's event loop on the DOM manipulation task source to fire the event inside that outer event loop. Does that make sense?
There was a problem hiding this comment.
Yeah the impl does go through Mojo. I wasn't sure how specific we had to be about how the event gets to its destination.
I am not familiar with the specifics of task sources and event loops yet, but I understand that ultimately the parent should be the one responsible for firing the event. In the impl, the IPC is received by the <fencedframe>, which essentially fires the event at itself. Does firing an event on an element inherently use the associated Window's event loop?
There was a problem hiding this comment.
No, firing an event at an element uses whatever event loop is on the stack. It's no different from calling a function really, and you can't call a JS function across documents that live in different processes.
That's why you have to queue a task from the child onto the parent's event loop, and the task will fire an event from entirely within the parent's context. Check out the usages of queue a task to see how to use it, or feel free to ask me specifics.
There was a problem hiding this comment.
I think I've figured out how to queue the event firing task in the unfenced parent's event loop, PTAL!
|
|
||
| 1. If any of the following conditions are met, throw a {{SecurityError}} {{DOMException}}: | ||
|
|
||
| * |navigable| is not a [=fenced navigable container/fenced navigable=]; |
There was a problem hiding this comment.
When is this hit? In an iframe inside of a fenced frame?
There was a problem hiding this comment.
We are only exposing this interface in fenced frame root documents, and IIUC URN iframes expose window.fence as well, so we shouldn't allow notifyEvent there.
There was a problem hiding this comment.
OK so this will throw a security error for both URN iframes, as well as iframes nested within fenced frames, right? And that's the intention?
There was a problem hiding this comment.
"This interface" is referring to fence.notifyEvent() specifically.
cross-origin iframes inside a fenced frame get access to window.fence. Is that true, and if so does it change the calculus here at all?
Step 4 of the create browsing context patch is the piece that has subframes inherit their creator's fenced frame config instance, which is then detected by the fence getter and ultimately lets subframes call window.fence. I don't think that fact changes the calculus of this algorithm, since we only want fenced frame roots to be able to call this method, which is what's currently happening.
There was a problem hiding this comment.
Yes, +1 to everything Liam said (sorry, I now realize my wording was a bit confusing).
spec.bs
Outdated
|
|
||
| 1. [=Consume user activation=] for [=this=]'s [=relevant global object=]. | ||
|
|
||
| 1. [=Fire an event=] named <code>[=fencedtreeclick=]</code> at |navigable|'s |
There was a problem hiding this comment.
No, firing an event at an element uses whatever event loop is on the stack. It's no different from calling a function really, and you can't call a JS function across documents that live in different processes.
That's why you have to queue a task from the child onto the parent's event loop, and the task will fire an event from entirely within the parent's context. Check out the usages of queue a task to see how to use it, or feel free to ask me specifics.
spec.bs
Outdated
| attributes to `true`. When running the | ||
| <a href="https://dom.spec.whatwg.org/#inner-event-creation-steps">inner event creation steps</a>, | ||
| set the <var ignore=''>time</var> to a user-agent-defined value that is consistent across all | ||
| invocations of this method. |
There was a problem hiding this comment.
Hmm, well the event should actually be created inside the parent asynchronously as part of the task queued by the child anyways, so I'm not sure how useful that is at all since the parent is ultimately in charge of firing the event. There is no information in the event that the parent doesn't already have access to.
This adds event handlers for `onfencedtreeclick`, and describes the algorithm for `window.fence.notifyEvent()`, which fires `fencedtreeclick` events from the fenced frame's document to the `HTMLFencedFrameElement` in the embedder.
|
FYI I had to force-push my feature branch after rebasing, which is why there are 5 new commits. There have been no new changes, just a rebase. Sorry for letting this sit, hoping to get back to it soon! |
spec.bs
Outdated
| attributes to `true`. When running the | ||
| <a href="https://dom.spec.whatwg.org/#inner-event-creation-steps">inner event creation steps</a>, | ||
| set the <var ignore=''>time</var> to a user-agent-defined value that is consistent across all | ||
| invocations of this method. |
There was a problem hiding this comment.
So what should the value actually be? Should we just make it always null or something? What does it mean for it to be consistent across all invocations of this method? Certainly a proper "time stamp" can't behave like that, right? (Hence why I ask if we should make it "null")
|
|
||
| 1. If any of the following conditions are met, throw a {{SecurityError}} {{DOMException}}: | ||
|
|
||
| * |navigable| is not a [=fenced navigable container/fenced navigable=]; |
There was a problem hiding this comment.
OK so this will throw a security error for both URN iframes, as well as iframes nested within fenced frames, right? And that's the intention?
This adds event handlers for
onfencedtreeclick, and describes the algorithm forwindow.fence.notifyEvent(), which firesfencedtreeclickevents from the fenced frame's document to theHTMLFencedFrameElementin the embedder.Preview | Diff