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

Issue 2507503002: Use touch events to report stylus events (Closed)

Created:
4 years, 1 month ago by denniskempin
Modified:
4 years ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, dtapuska+blinkwatch_chromium.org, tdresser+watch_chromium.org, eae+blinkwatch, sof, jam, blink-reviews-dom_chromium.org, dglazkov+blink, darin-cc_chromium.org, blink-reviews, kalyank, kinuko+watch, ozone-reviews_chromium.org, rwlbuis, jdufault, xiaoyinh(OOO Sep 11-29)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use touch events to report stylus events UI review has decided that a stylus should behave like touch and not like mouse. e.g. It should scroll when dragging, not select text. This change will switch stylus events back to use TouchEvents instead of MouseEvents. Besides adjusting the evdev converter to report TouchEvents, this change also updates the routing of touch events to carry along pointer type information. BUG=665499 TEST=Test with rbyers eventTest and paint app Committed: https://crrev.com/1f1f36a66ac46e957542f51e3c74bfcae70a5f36 Cr-Commit-Position: refs/heads/master@{#435479}

Patch Set 1 #

Total comments: 4

Patch Set 2 : fixed unit test #

Total comments: 4

Patch Set 3 : only pass pointer type through plugin api #

Patch Set 4 : fixed conversion of pointer type for touch end events #

Patch Set 5 : fixed nit #

Total comments: 1

Patch Set 6 : fixed nits #

Patch Set 7 : Added check for pointerType in WebInputEventConversionTest #

Patch Set 8 : fixed nit #

Patch Set 9 : I should pay attention... #

Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -229 lines) Patch
M content/renderer/pepper/event_conversion.cc View 1 2 6 chunks +13 lines, -73 lines 0 comments Download
M third_party/WebKit/Source/core/events/TouchEvent.h View 1 2 5 chunks +14 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/events/TouchEvent.cpp View 1 2 3 chunks +6 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/input/TouchEventManager.cpp View 1 2 3 chunks +12 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/web/WebInputEventConversion.cpp View 1 2 3 4 4 chunks +18 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp View 1 2 3 4 5 6 7 8 9 chunks +18 lines, -8 lines 0 comments Download
M ui/events/ozone/evdev/touch_evdev_types.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M ui/events/ozone/evdev/touch_event_converter_evdev.h View 1 2 3 4 5 1 chunk +0 lines, -6 lines 0 comments Download
M ui/events/ozone/evdev/touch_event_converter_evdev.cc View 1 2 3 4 5 5 chunks +18 lines, -53 lines 0 comments Download
M ui/events/ozone/evdev/touch_event_converter_evdev_unittest.cc View 1 2 chunks +22 lines, -62 lines 0 comments Download

Messages

Total messages: 39 (15 generated)
denniskempin
Hi guys! please have a look at this change and let me know what you ...
4 years, 1 month ago (2016-11-15 18:31:53 UTC) #2
Navid Zolghadr
https://codereview.chromium.org/2507503002/diff/1/third_party/WebKit/Source/core/dom/Touch.cpp File third_party/WebKit/Source/core/dom/Touch.cpp (right): https://codereview.chromium.org/2507503002/diff/1/third_party/WebKit/Source/core/dom/Touch.cpp#newcode59 third_party/WebKit/Source/core/dom/Touch.cpp:59: m_pointerProperties(properties) { It seems that everytime we create a ...
4 years, 1 month ago (2016-11-15 18:53:38 UTC) #4
denniskempin
On 2016/11/15 18:53:38, Navid Zolghadr wrote: > https://codereview.chromium.org/2507503002/diff/1/third_party/WebKit/Source/core/dom/Touch.cpp > File third_party/WebKit/Source/core/dom/Touch.cpp (right): > > https://codereview.chromium.org/2507503002/diff/1/third_party/WebKit/Source/core/dom/Touch.cpp#newcode59 ...
4 years, 1 month ago (2016-11-17 18:07:12 UTC) #5
spang
Please write some logs based tests. Something is really wrong if you can make a ...
4 years, 1 month ago (2016-11-17 18:36:17 UTC) #6
Navid Zolghadr
On 2016/11/17 18:07:12, denniskempin wrote: > On 2016/11/15 18:53:38, Navid Zolghadr wrote: > > > ...
4 years, 1 month ago (2016-11-17 19:30:32 UTC) #7
mustaq
Sorry Dennis, I missed reviewing your CL yesterday as I had planned. Could you please ...
4 years, 1 month ago (2016-11-17 21:24:57 UTC) #9
denniskempin
On 2016/11/17 21:24:57, mustaq wrote: > Sorry Dennis, I missed reviewing your CL yesterday as ...
4 years, 1 month ago (2016-11-17 23:10:06 UTC) #10
denniskempin
https://codereview.chromium.org/2507503002/diff/20001/third_party/WebKit/Source/core/dom/Touch.h File third_party/WebKit/Source/core/dom/Touch.h (right): https://codereview.chromium.org/2507503002/diff/20001/third_party/WebKit/Source/core/dom/Touch.h#newcode112 third_party/WebKit/Source/core/dom/Touch.h:112: const WebPointerProperties&); On 2016/11/17 21:24:57, mustaq wrote: > I ...
4 years ago (2016-11-28 19:40:48 UTC) #11
mustaq
I understand it is meant to be a temporary change but I am not convinced ...
4 years ago (2016-11-28 21:13:52 UTC) #12
blink-reviews
We need stylus events in PPAPI, and I've been trying to follow the flow of ...
4 years ago (2016-11-28 21:41:13 UTC) #13
chromium-reviews
We need stylus events in PPAPI, and I've been trying to follow the flow of ...
4 years ago (2016-11-28 21:41:13 UTC) #14
mustaq
After an offline discussion, I realized that the plugin we are considering here only needs ...
4 years ago (2016-11-29 21:55:34 UTC) #15
denniskempin
Mustaq, let me know what you think! Michael and Elliott, could you have a look ...
4 years ago (2016-11-30 00:31:36 UTC) #16
mustaq
On 2016/11/30 00:31:36, denniskempin wrote: > Mustaq, let me know what you think! > > ...
4 years ago (2016-11-30 16:27:36 UTC) #17
Navid Zolghadr
lgtm https://codereview.chromium.org/2507503002/diff/80001/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp (right): https://codereview.chromium.org/2507503002/diff/80001/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp#newcode153 third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:153: WebPointerProperties::PointerType::Touch); nit: I know it is just a ...
4 years ago (2016-11-30 16:37:03 UTC) #18
spang
lgtm
4 years ago (2016-11-30 18:55:28 UTC) #19
esprehn
Can you explain more in the description why this is desirable? The change lgtm, but ...
4 years ago (2016-11-30 19:08:21 UTC) #20
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/2507503002/120001
4 years ago (2016-11-30 20:38:10 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/345336)
4 years ago (2016-11-30 20:57:44 UTC) #26
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/2507503002/140001
4 years ago (2016-11-30 21:13:35 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/345379)
4 years ago (2016-11-30 21:41:17 UTC) #31
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/2507503002/160001
4 years ago (2016-11-30 22:00:34 UTC) #34
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years ago (2016-11-30 23:40:16 UTC) #37
commit-bot: I haz the power
4 years ago (2016-11-30 23:42:16 UTC) #39
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/1f1f36a66ac46e957542f51e3c74bfcae70a5f36
Cr-Commit-Position: refs/heads/master@{#435479}

Powered by Google App Engine
This is Rietveld 408576698