window.event and shadow trees

[annevk]

In https://chromium-review.googlesource.com/c/chromium/src/+/1177406 Yuki Yamada corrected a wpt test that now illustrates that if a listener of a node in a shadow tree runs for a composed event, window.event will be undefined.

If this listener were to call a function that's not aware of the shadow tree, but does keep around state about the composed event, it'll learn about the existence of the shadow tree.

I think this is only observable with web developer-implemented shadow trees, not UA-implemented shadow trees, but it still seems somewhat suboptimal? Am I missing something here?

cc @whatwg/components

[smaug----]

Hmm, that particular test isn't updated yet in mozilla-central, and currently it is marked as failing
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/dom/events/event-global-extra.window.js.ini
So I assume that test starts to pass once merged.

[annevk]

It only just merged on the Chromium side so I suspect it'll take a while to propagate all the way. I thought that Fx hadn't accounted for shadow tree handling yet when it came to window.event, but maybe I'm wrong about that.

[smaug----]

@hsivonen did add support for shadow DOM too.

[yuki3]

AFAIK, the idea to attempt to hide window.event in a shadow tree started at Issue 334's comment. I talked offline with Hayato and he shares the same concern with Anne, without having a strong motivation to hide window.event in a shadow tree no longer.

I wasn't aware of this issue when we discussed Issue 334, and I'm now leaning to the idea to expose window.event regardless of a shadow tree.

[hayatoito]

Thanks @yuki3

FWIW, Blink changed the behavior of window.event for Shadow DOM v1 here https://chromium-review.googlesource.com/c/chromium/src/+/773922. We've not heard any web site breakage report by this change.

Given that, I guess the risk of changing window.event behavior for a shadow tree is (still) low enough, and any change doesn't matter much for web developers in practice, I am hoping.

On the implementer hat, it could make the implementation much simpler if we don't try to hide window.event in a shadow tree, per @yuki3 .

[annevk]

What I'm proposing is slightly different I think. Make window.event undefined if the event is not composed, but expose the Event object if the event is composed, as the event object will then be reachable outside the shadow tree anyway.

[rniwa]

FWIW, the latest betas of iOS 12 and macOS Mojave hides window.event when the target is in the shadow tree (matching behaviors of Firefox & Chrome in this regard for now). The change proposed by @annevk makes sense to me though.

[yuki3]

I'm afraid that I've never been understanding why we want to hide (or set to undefined) window.event in a shadow tree. May I ask someone to elaborate why? Simply thinking, less difference, better?

[annevk]

@yuki3 the rationale is that if an event listener in a shadow tree calls a "global function", that global function then has access to the shadow tree through window.event, breaking encapsulation.

Technically this applies to composed events as well, however in that case the global function could have obtained a reference to the Event object earlier on easily enough as well.

[yuki3]

Ah, now I see that we're afraid of a leak through event.{target,srcElement,currentTarget,...}.

As I'm newbie about shadow trees, I don't yet understand how much the encapsulation is important. However, if we suppose that the encapsulation is very important, I'd think that hiding window.event.target is more important. Detection of existence of shadow trees seems less impactful/harmful than exposure of shadow DOM nodes.

[annevk]

We do adjust e.target (see https://dom.spec.whatwg.org/#concept-event-listener-invoke for details) during dispatch, but in the specific case of a shadow tree listener invoking a global function it will leak if either the global function already had access to e (capture listener) or gets it from window.event.

[yuki3]

e.target is somewhat adjusted, but e.currentTarget, etc. do not seem adjusted. So, IIUC what you're saying is, the following is possible if window.event were exposed in a shadow tree, I guess?

function global_function() {
  console.log(window.event);
  console.log(window.event.currentTarget);  // => shadowNode
}

shadowNode.addEventListener(type, (e) => {
  global_function();
});

Is this what we're afraid of?

[annevk]

Yeah, though for that scenario e.target would also expose it (assuming the shadow tree node is the target).

[yuki3]

Thanks. I think I've finally caught up to your first question.

In https://chromium-review.googlesource.com/c/chromium/src/+/1177406 Yuki Yamada corrected a wpt test that now illustrates that if a listener of a node in a shadow tree runs for a composed event, window.event will be undefined.

If this listener were to call a function that's not aware of the shadow tree, but does keep around state about the composed event, it'll learn about the existence of the shadow tree.

Supposing that author knows the composed event and the target where the event was initially dispatched (i.e. target.dispatchEvent(e)), it seems possible to me that author looks for e.currentTarget in a subtree under the target. If author was not able to find e.currentTarget in the subtree, it must be a shadow node, mustn't it?

If so, tweaking window.event wouldn't help very much, I think.

[annevk]

In case of a single developer that already has access to the shadow tree, hiding it indeed doesn't help.

The encapsulation comes into play with libraries not aware of the shadow trees reacting to UA-dispatched events or events dispatched by libraries aware of the shadow trees.

[yuki3]

Could you give me an example code of the case that we are able to detect a shadow node only by window.event but no other way?

[annevk]

I guess this is the point where I clarify it's not a hard guarantee and we don't intend to protect against those messing around with prototypes and such?

[yuki3]

Ah, I see.

[annevk]

To make this more concrete, I think all that would change here is step 2.8.2 of https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke from

If tuple’s item-in-shadow-tree is false, then set global’s current event to event.

to

If tuple’s item-in-shadow-tree is false or event's composed flag is set, then set global’s current event to event.

(tuple above will become struct per #686 so we might want to wait on that before landing this change.)

[annevk]

F2F: it's observable either way and we'd like the window.event experience to be worse, so we'll preserve the status quo.