blink::Event::target() returns null during invoking it |
|||
Issue descriptionblink::Event::target() returns null on invoking blink::JSBasedEventListener::handleEvent(). Currently this is handled with an early return, but an event's target should not null on invoking that as written in step 1 of "invoke" in DOM standard: https://dom.spec.whatwg.org/#concept-event-listener-invoke This problem is found in another issue: https://bugs.chromium.org/p/chromium/issues/detail?id=892970
,
Oct 9
Is this a regression caused by https://chromium.googlesource.com/chromium/src/+/1b2f95835607dbb9c6021df8893fb18f4c7aee9e? Could you confirm that?
,
Oct 9
No. This has been a problem in original code, and the CL you mentioned just revealed it. I 'll try to address this issue, but maybe I don't have enough time unfortunately because this is my final week of my internship :(
,
Oct 9
I've added the comment there: https://chromium-review.googlesource.com/c/chromium/src/+/1270596#message-ec673e8343c2b5e9ee6ccc04217b4aa5aef0b398 It is still unclear to me the CL is a regression or not. We might want to make it clear why cluster-fuzz detected the CL as a regression.
,
Oct 9
I have confirmed that the reproducer test case caused the crash also in the commit 2555486fd96872f6b98f9ad54c541d7ee2b6fcb1, which is just before the commit 1b2f95835607dbb9c6021df8893fb18f4c7aee9e. The CL crbug.com/892970 is reported on: https://chromium-review.googlesource.com/c/chromium/src/+/1215512 The CL just before that: https://chromium-review.googlesource.com/c/chromium/src/+/1254145
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f594c634639b4524dab11fee8dac11f34bf88671 commit f594c634639b4524dab11fee8dac11f34bf88671 Author: Yuki Yamada <yukiy@google.com> Date: Wed Oct 10 06:47:13 2018 Add an early return for null target On dispatching an event, current implementation of blink::Event::target() can return null while standard says that target should not be null. This CL's modification is just temporary bug fix, so this issue should be addressed in other CLs. Bug: 893449 , 892970 Change-Id: I4aaa94f14e12e9100ad901ce1b7bc23230792495 Reviewed-on: https://chromium-review.googlesource.com/c/1270596 Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Commit-Queue: Yuki Yamada <yukiy@google.com> Cr-Commit-Position: refs/heads/master@{#598232} [modify] https://crrev.com/f594c634639b4524dab11fee8dac11f34bf88671/third_party/blink/renderer/bindings/core/v8/js_based_event_listener.cc
,
Oct 11
,
Nov 6
https://chromium-review.googlesource.com/c/chromium/src/+/1272937 fixed this.
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ed9605b4c99774d8865dbd190e13c9fe5c2d03a commit 2ed9605b4c99774d8865dbd190e13c9fe5c2d03a Author: Hayato Ito <hayato@chromium.org> Date: Fri Nov 09 17:31:43 2018 Fix a bug in https://crrev.com/c/1272937, where event.target can be still null Fix a bug in the previous CL, https://crrev.com/c/1272937, where event.target can be null when GetElement() returns nullptr. Bug: 892970 , 893449 , 902287 Change-Id: I7a08227d39117c2dc90fe720f0d6ffd62d9b2ea6 Reviewed-on: https://chromium-review.googlesource.com/c/1322177 Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#606884} [modify] https://crrev.com/2ed9605b4c99774d8865dbd190e13c9fe5c2d03a/third_party/blink/renderer/modules/accessibility/ax_object.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by yukiy@google.com
, Oct 9