New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 779461 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove window.event for Shadow DOM

Project Member Reported by hayato@chromium.org, Oct 30 2017

Issue description

The contex is: https://github.com/whatwg/dom/issues/334
We are trying to standardize certain Microsoft event extensions. One of them is window.event. Blink already *supports* window.event.

In the discussion, we agreed that window.event should be undefined if an Event's target is in a shadow tree.
That is effectivery equivalent to *removing* window.event for Shadow DOM.


 

Comment 1 by hayato@chromium.org, Oct 30 2017

Entry on feature dashboard: https://www.chromestatus.com/features/5084727350394880

Comment 2 by hayato@chromium.org, Nov 15 2017

Cc: hayato@chromium.org peria@chromium.org kochi@chromium.org yukishiino@chromium.org
Owner: rakina@chromium.org
"Intent to remove" got 3 LGTMs, with a slightly different approach. We only *remove* window.event for v1, and add use counters for the usage of v0.

See https://groups.google.com/a/chromium.org/d/msg/blink-dev/a9SmvyKUX4w/Yi1a8GlXBQAJ

rakina@, could you work on this? You would learn how to add use counters.
If you need any help from binding team, though I don't think we don't need it anymore, please feel free to ask binding-team. :)

kochi@, please feel free to help rakina@.

cc: +yukishiino, +peria, just in case.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7997dbe61622af78d36da362c0d5c1049e364d49

commit 7997dbe61622af78d36da362c0d5c1049e364d49
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Tue Nov 21 18:51:02 2017

Remove window.event support for V1 ShadowDOM

window.event should be undefined if an Event's target is in a shadow
tree, because it is leaking information from inside the shadow tree.

This change will remove window.event support for events targeted to
nodes within a V1 shadow tree.

Discussion link: https://github.com/whatwg/dom/issues/334

Bug: 779461
Change-Id: I310bc72463369befd693de9321a9949a97e83f5e
Reviewed-on: https://chromium-review.googlesource.com/773922
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518317}
[modify] https://crrev.com/7997dbe61622af78d36da362c0d5c1049e364d49/third_party/WebKit/LayoutTests/shadow-dom/slotchange.html
[add] https://crrev.com/7997dbe61622af78d36da362c0d5c1049e364d49/third_party/WebKit/LayoutTests/shadow-dom/window-event.html
[modify] https://crrev.com/7997dbe61622af78d36da362c0d5c1049e364d49/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54639c022f3ac72bba48acb9310f38f0d4ac6a01

commit 54639c022f3ac72bba48acb9310f38f0d4ac6a01
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Wed Nov 22 08:28:40 2017

Add use counter to track window.event usage for V0 Shadow DOM

window.event should be undefined if an Event's target is in a shadow
tree, because it is leaking information from inside the shadow tree.
But we don't know how frequently window.event is used for V0 Shadow DOM.

This change adds a use counter for window.event calls where the event's
target is in a V0 shadow tree,  so that we will know when usage is low
and it's safe to remove window.event for V0 Shadow DOM. Support for
window.event for V1 Shadow DOM is removed in crrev.com/c/773922

Discussion link: https://github.com/whatwg/dom/issues/334

Bug: 779461
Change-Id: Iadc734ebaeb90f44f832bcbe30f0ac9d0be1cff1
Reviewed-on: https://chromium-review.googlesource.com/780722
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518567}
[modify] https://crrev.com/54639c022f3ac72bba48acb9310f38f0d4ac6a01/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
[modify] https://crrev.com/54639c022f3ac72bba48acb9310f38f0d4ac6a01/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/54639c022f3ac72bba48acb9310f38f0d4ac6a01/tools/metrics/histograms/enums.xml

Comment 5 by kochi@chromium.org, Nov 29 2017

The remaining task for this is to watch for V0 stats and remove
for V0 eventually?

Comment 6 by kochi@chromium.org, Nov 29 2017

Related:  issue 223749  (general window.event interop issue)
Re #5: I think so.
Labels: Hotlist-Interop

Sign in to add a comment