Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Issue 2408133007: Check the root frame pointer events as well for capture (Closed)

Created:
4 years, 2 months ago by Navid Zolghadr
Modified:
4 years, 2 months ago
Reviewers:
dtapuska, mustaq, bokan
CC:
chromium-reviews, blink-reviews, nzolghadr+blinkwatch_chromium.org, dtapuska+blinkwatch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Check the root pointer events as well for capture Touch pointer events events are being routed to the target directly without passing all the iframes' EventHandlers. As a result the inner EventHandler has no clue whether a given pointerId is active or capturable if that is a touch pointer event. This CL adds the check to every iframe to also checks the root frame for the pointer event and it allows the capture if that pointer event was sent to a node inside the requested iframe. BUG=653860 Committed: https://crrev.com/85c41444f11d3c24ea24d3c776d96dbe913cad03 Cr-Commit-Position: refs/heads/master@{#426205}

Patch Set 1 #

Total comments: 14

Patch Set 2 : Applying Mustaq's comments #

Total comments: 2

Patch Set 3 : Only lookup the root frame for touch pointer events #

Total comments: 4

Patch Set 4 : adding comments #

Total comments: 2

Patch Set 5 : Fix the function naming #

Messages

Total messages: 47 (30 generated)
Navid Zolghadr
4 years, 2 months ago (2016-10-13 15:53:03 UTC) #5
mustaq
https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/events/PointerEventFactory.cpp File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right): https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/events/PointerEventFactory.cpp#newcode352 third_party/WebKit/Source/core/events/PointerEventFactory.cpp:352: int type = p.pointerTypeInt(); Nit: s/type/typeInt One more below. ...
4 years, 2 months ago (2016-10-13 16:38:25 UTC) #6
Navid Zolghadr
ptal https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/events/PointerEventFactory.cpp File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right): https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/events/PointerEventFactory.cpp#newcode352 third_party/WebKit/Source/core/events/PointerEventFactory.cpp:352: int type = p.pointerTypeInt(); On 2016/10/13 16:38:25, mustaq ...
4 years, 2 months ago (2016-10-13 17:32:32 UTC) #11
mustaq
https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/input/EventHandler.cpp File third_party/WebKit/Source/core/input/EventHandler.cpp (right): https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/input/EventHandler.cpp#newcode1202 third_party/WebKit/Source/core/input/EventHandler.cpp:1202: if (m_frame != m_frame->localFrameRoot() && On 2016/10/13 17:32:32, Navid ...
4 years, 2 months ago (2016-10-13 19:55:38 UTC) #14
Navid Zolghadr
ptal https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/input/EventHandler.cpp File third_party/WebKit/Source/core/input/EventHandler.cpp (right): https://codereview.chromium.org/2408133007/diff/1/third_party/WebKit/Source/core/input/EventHandler.cpp#newcode1202 third_party/WebKit/Source/core/input/EventHandler.cpp:1202: if (m_frame != m_frame->localFrameRoot() && On 2016/10/13 19:55:38, ...
4 years, 2 months ago (2016-10-14 14:50:28 UTC) #17
mustaq
lgtm https://codereview.chromium.org/2408133007/diff/40001/third_party/WebKit/Source/core/events/PointerEventFactory.h File third_party/WebKit/Source/core/events/PointerEventFactory.h (right): https://codereview.chromium.org/2408133007/diff/40001/third_party/WebKit/Source/core/events/PointerEventFactory.h#newcode76 third_party/WebKit/Source/core/events/PointerEventFactory.h:76: WebPointerProperties::PointerType getPointerType(int pointerId) const; Document that it returns ...
4 years, 2 months ago (2016-10-17 20:18:56 UTC) #20
Navid Zolghadr
https://codereview.chromium.org/2408133007/diff/40001/third_party/WebKit/Source/core/events/PointerEventFactory.h File third_party/WebKit/Source/core/events/PointerEventFactory.h (right): https://codereview.chromium.org/2408133007/diff/40001/third_party/WebKit/Source/core/events/PointerEventFactory.h#newcode76 third_party/WebKit/Source/core/events/PointerEventFactory.h:76: WebPointerProperties::PointerType getPointerType(int pointerId) const; On 2016/10/17 20:18:56, mustaq wrote: ...
4 years, 2 months ago (2016-10-18 14:18:11 UTC) #23
Navid Zolghadr
4 years, 2 months ago (2016-10-18 14:39:18 UTC) #25
bokan
lgtm
4 years, 2 months ago (2016-10-18 16:56:52 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2408133007/60001
4 years, 2 months ago (2016-10-18 17:32:22 UTC) #31
commit-bot: I haz the power
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_presubmit/builds/283746)
4 years, 2 months ago (2016-10-18 17:50:11 UTC) #33
dtapuska
lgtm % nit https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h File third_party/WebKit/Source/core/input/EventHandler.h (right): https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h#newcode341 third_party/WebKit/Source/core/input/EventHandler.h:341: bool RootFrameTouchPointerActiveInCurrentFrame(int pointerId) const; I don't ...
4 years, 2 months ago (2016-10-19 00:22:29 UTC) #34
dtapuska
lgtm % nit https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h File third_party/WebKit/Source/core/input/EventHandler.h (right): https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h#newcode341 third_party/WebKit/Source/core/input/EventHandler.h:341: bool RootFrameTouchPointerActiveInCurrentFrame(int pointerId) const; I don't ...
4 years, 2 months ago (2016-10-19 00:22:29 UTC) #35
Navid Zolghadr
https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h File third_party/WebKit/Source/core/input/EventHandler.h (right): https://codereview.chromium.org/2408133007/diff/60001/third_party/WebKit/Source/core/input/EventHandler.h#newcode341 third_party/WebKit/Source/core/input/EventHandler.h:341: bool RootFrameTouchPointerActiveInCurrentFrame(int pointerId) const; On 2016/10/19 00:22:29, dtapuska wrote: ...
4 years, 2 months ago (2016-10-19 15:35:37 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2408133007/80001
4 years, 2 months ago (2016-10-19 15:36:08 UTC) #43
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 2 months ago (2016-10-19 15:49:23 UTC) #45
commit-bot: I haz the power
4 years, 2 months ago (2016-10-21 13:08:52 UTC) #47
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/85c41444f11d3c24ea24d3c776d96dbe913cad03
Cr-Commit-Position: refs/heads/master@{#426205}

Powered by Google App Engine
This is Rietveld 408576698