Support event handling in SVG's use-element shadow trees

Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
See also the following CLs as necessary changes before landing this CL:
- https://codereview.chromium.org/2537143004
- https://codereview.chromium.org/2752763002

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
Review-Url: https://codereview.chromium.org/2742293006
Cr-Commit-Position: refs/heads/master@{#457705}
Committed: https://chromium.googlesource.com/chromium/src/+/b586056392985ae99a88b83e66a4685e356020d2

[hayato, 2017-03-15 05:33:50]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 05:34:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 07:04:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 07:04:38]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hayato, 2017-03-15 08:11:03]

user agent shadow root -> closed shadow root

[hayato, 2017-03-15 08:11:05]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 08:11:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 08:13:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 08:13:32]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[hayato, 2017-03-15 08:39:54]

Description was changed from

==========
Fix svg <use>

BUG=667324
==========

to

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-elemetn shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
The followings are necessary changes before re-supporting event handling.
- TODO
- TODO

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

[hayato, 2017-03-15 08:40:04]

Description was changed from

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-elemetn shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
The followings are necessary changes before re-supporting event handling.
- TODO
- TODO

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

to

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
The followings are necessary changes before re-supporting event handling.
- TODO
- TODO

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

[hayato, 2017-03-15 09:15:19]

rebased

[hayato, 2017-03-15 09:15:21]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 09:15:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hayato, 2017-03-15 10:53:25]

https://codereview.chromium.org/2752763002 is separated CL from this CL.

[commit-bot: I haz the power, 2017-03-15 11:01:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 11:01:54]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hayato, 2017-03-15 11:19:55]

Description was changed from

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
The followings are necessary changes before re-supporting event handling.
- TODO
- TODO

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

to

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
See also the following CLs as necessary changes before landing this CL:
- https://codereview.chromium.org/2537143004
- https://codereview.chromium.org/2752763002

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

[hayato, 2017-03-15 11:23:05]

split and rebased

[hayato, 2017-03-15 11:23:06]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 11:23:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hayato, 2017-03-15 11:26:32]

hayato@chromium.org changed reviewers:
+ fs@opera.com, pdr@chromium.org

[hayato, 2017-03-15 11:26:33]

PTAL

This CL depends on https://codereview.chromium.org/2742293006.

[commit-bot: I haz the power, 2017-03-15 12:49:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 12:49:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fs, 2017-03-15 13:28:12]

This looks good to me, with a few comments and suggested test improvements.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg
(right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg:1:
<svg version="1.1" baseProfile="basic" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintAndPixelTest()">
Any particular reason why these tests (this and the following two) can't just
use HTML wrapping and testharness.js? (I don't see a need for using
text-based-repaint.js here.)

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/custom/use-instanceRoot-event-listener-liveness.xhtml
(right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/custom/use-instanceRoot-event-listener-liveness.xhtml:33:
var event = document.createEvent("MouseEvents");
Why convert this away from the constructor form?

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h (right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h:84: // that the
SVG code does not add the event handler in both
This comment reads a bit backwards to me ("implemented in JavaScript" - should
that be "added by JavaScript? still missing a negation in that case.)

Maybe "(Needs to )return true, so that event handlers from markup are not cloned
twice when building the shadow tree for SVGUseElements."

(Also, "the SVG code" is just generally vague, and referencing method name is
sensitive to renames - as the next comment seem to imply...)

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h:86: //
SVGUseElement::transferEventListenersToShadowTree
I guess this should say cloneNonMarkupEventListeners or so instead?

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/EventListenerMap.cpp (right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/EventListenerMap.cpp:194: // shadow tree
during cloning.
This comment doesn't make a lot of sense in this context. Would make more sense
before the calls to SVGUseElement::cloneNonMarkupEventListeners I think. The
method name should suffice as documentation here I think, possibly with a short
comment (which I suspect will be redundant with the name...)

(Also, transfered -> transferred)

[hayato, 2017-03-16 05:58:07]

Addressed

[hayato, 2017-03-16 05:58:09]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 05:58:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hayato, 2017-03-16 05:59:23]

fs@, thank you for the review.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg
(right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg:1:
<svg version="1.1" baseProfile="basic" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintAndPixelTest()">
Ah, these tests are just the result of re-landing tests which were removed (or
were updated to drop the support event handling) in
https://codereview.chromium.org/2186823002.
I tried to recover the original tests as they were in this CL, intentionally,
without any updates, to isolate issues.

I agree that there are some old-fashioned things in these tests, but it might be
better not to update these tests together with this CL.
We might want to update these tests in another CL, as a separate concern.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/custom/use-instanceRoot-event-listener-liveness.xhtml
(right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/custom/use-instanceRoot-event-listener-liveness.xhtml:33:
var event = document.createEvent("MouseEvents");
Yeah, I do not have to revert my previous change here. That should not change
the test's behavior. Done.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h (right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h:84: // that the
SVG code does not add the event handler in both
Yeah, sorry, I should have taken a look at comment carefully. I have just
*recovered* the original comment without any attention.

Done.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.h:86: //
SVGUseElement::transferEventListenersToShadowTree
I have removed this comment because it does not give us much benefits, I think.

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/EventListenerMap.cpp (right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/EventListenerMap.cpp:194: // shadow tree
during cloning.
Yeah, let me remove the comment. Done.

[commit-bot: I haz the power, 2017-03-16 06:34:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 06:34:50]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[fs, 2017-03-16 08:43:13]

lgtm

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg
(right):

https://codereview.chromium.org/2742293006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/custom/use-event-handler-on-referenced-element.svg:1:
<svg version="1.1" baseProfile="basic" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintAndPixelTest()">
On 2017/03/16 at 05:59:22, hayato wrote:
> Ah, these tests are just the result of re-landing tests which were removed (or
were updated to drop the support event handling) in
https://codereview.chromium.org/2186823002.
> I tried to recover the original tests as they were in this CL, intentionally,
without any updates, to isolate issues.
> 
> I agree that there are some old-fashioned things in these tests, but it might
be better not to update these tests together with this CL.
> We might want to update these tests in another CL, as a separate concern.

SGTM.

[hayato, 2017-03-16 10:18:13]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-03-16 10:18:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 10:24:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 10:24:41]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hayato, 2017-03-16 10:26:33]

hayato@chromium.org changed reviewers:
+ tkent@chromium.org

[hayato, 2017-03-16 10:26:34]

tkent@,
Could you stamp for bindings/core/v8/V8LazyEventListener.h?

[tkent, 2017-03-16 23:22:35]

On 2017/03/16 at 10:26:34, hayato wrote:
> tkent@,
> Could you stamp for bindings/core/v8/V8LazyEventListener.h?

lgtm.

use-event-handler-on-referenced-element.svg is failing on all platforms.

[hayato, 2017-03-17 04:00:12]

On 2017/03/16 at 23:22:35, tkent wrote:
> On 2017/03/16 at 10:26:34, hayato wrote:
> > tkent@,
> > Could you stamp for bindings/core/v8/V8LazyEventListener.h?
> 
> lgtm.
> 
> use-event-handler-on-referenced-element.svg is failing on all platforms.

It looks use-event-handler-on-referenced-element.svg is a pixel test. Let me
land with this CL with rebasing.

[hayato, 2017-03-17 04:03:45]

needs rebaseline

[hayato, 2017-03-17 04:03:47]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 04:04:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hayato, 2017-03-17 04:04:19]

The CQ bit was unchecked by hayato@chromium.org

[hayato, 2017-03-17 04:04:21]

The CQ bit was checked by hayato@chromium.org

[hayato, 2017-03-17 04:04:22]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
fs@opera.com
Link to the patchset: https://codereview.chromium.org/2742293006/#ps100001
(title: "needs rebaseline")

[commit-bot: I haz the power, 2017-03-17 04:04:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 07:08:40]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1489723461823750,
"parent_rev": "127e697b3d265b427a83a9e82cb5fcc2ef12d395", "commit_rev":
"b586056392985ae99a88b83e66a4685e356020d2"}

[commit-bot: I haz the power, 2017-03-17 07:09:24]

Description was changed from

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
See also the following CLs as necessary changes before landing this CL:
- https://codereview.chromium.org/2537143004
- https://codereview.chromium.org/2752763002

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324
==========

to

==========
Support event handling in SVG's use-element shadow trees

The spec is: https://svgwg.org/svg2-draft/struct.html#UseShadowTree (5.6.1. The
use-element shadow tree)

This CL is basically a fix for a regression, which was intentionally introduced
in https://codereview.chromium.org/2186823002, where I removed the code to
prevent universal XSS.

The most part of this CL is a re-land of the code which was removed there.
See also the following CLs as necessary changes before landing this CL:
- https://codereview.chromium.org/2537143004
- https://codereview.chromium.org/2752763002

Regarding, event#currentTarget(), the current spec is unclear.  So, this CL
honored the original behavior.

BUG=667324

Review-Url: https://codereview.chromium.org/2742293006
Cr-Commit-Position: refs/heads/master@{#457705}
Committed:
https://chromium.googlesource.com/chromium/src/+/b586056392985ae99a88b83e66a4...
==========

[commit-bot: I haz the power, 2017-03-17 07:09:26]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b586056392985ae99a88b83e66a4...