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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 19805005: Refactored touch-events flag test, correcting "enabled" logic. (Closed)

Created:
7 years, 5 months ago by girard
Modified:
7 years, 4 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, tfarina, jam, penghuang+watch_chromium.org, joi+watch-content_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, James Su, sadrul, flackr
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Refactored touch-events flag test, correcting "enabled" logic. We previously ignored the "enabled" flag if there was no touch device detected. We also ignored "disabled" in many places, so that we always ended up with touch if there was a touch device. Also, remove reference to gestures - no longer reachable. BUG=176058 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214535

Patch Set 1 #

Total comments: 2

Patch Set 2 : Refactor to use WebkitPrefs. #

Patch Set 3 : Add touch_device, refactor references, re-enable gestures, enforce touch flag. #

Patch Set 4 : Remove gesture logic. #

Patch Set 5 : Cleanup #

Total comments: 2

Patch Set 6 : Renamed IsTouchEnabled to AreTouchEventsEnabled. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -52 lines) Patch
M chrome/browser/ui/views/omnibox/omnibox_view_win.cc View 1 2 3 4 5 2 chunks +3 lines, -1 line 0 comments Download
M chrome/chrome.user32.delay.imports View 3 4 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_view_win.cc View 1 2 3 4 5 5 chunks +6 lines, -31 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 5 3 chunks +5 lines, -17 lines 0 comments Download
A ui/base/touch/touch_enabled.h View 1 2 3 4 5 1 chunk +18 lines, -0 lines 0 comments Download
A ui/base/touch/touch_enabled.cc View 1 2 3 4 5 1 chunk +30 lines, -0 lines 0 comments Download
M ui/ui.gyp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
girard
7 years, 5 months ago (2013-07-24 17:48:39 UTC) #1
flackr
https://codereview.chromium.org/19805005/diff/1/content/browser/renderer_host/render_widget_host_view_win.cc File content/browser/renderer_host/render_widget_host_view_win.cc (right): https://codereview.chromium.org/19805005/diff/1/content/browser/renderer_host/render_widget_host_view_win.cc#newcode431 content/browser/renderer_host/render_widget_host_view_win.cc:431: gesture_recognizer_(ui::GestureRecognizer::Create(this)) { Rather than duplicate the touch events settting, ...
7 years, 5 months ago (2013-07-24 18:41:52 UTC) #2
Ben Goodger (Google)
https://codereview.chromium.org/19805005/diff/1/content/browser/renderer_host/render_widget_host_view_win.cc File content/browser/renderer_host/render_widget_host_view_win.cc (right): https://codereview.chromium.org/19805005/diff/1/content/browser/renderer_host/render_widget_host_view_win.cc#newcode342 content/browser/renderer_host/render_widget_host_view_win.cc:342: else if (cmdline->GetSwitchValueASCII(switches::kTouchEvents) == no else after return
7 years, 5 months ago (2013-07-24 19:26:13 UTC) #3
girard
Cleaned up a lot of code - there were two places that evaluated the touch-enabled ...
7 years, 5 months ago (2013-07-26 15:54:58 UTC) #4
flackr
LGTM with nit. https://codereview.chromium.org/19805005/diff/23002/ui/base/touch/touch_enabled.h File ui/base/touch/touch_enabled.h (right): https://codereview.chromium.org/19805005/diff/23002/ui/base/touch/touch_enabled.h#newcode14 ui/base/touch/touch_enabled.h:14: UI_EXPORT bool IsTouchEnabled(); nit: This isn't ...
7 years, 5 months ago (2013-07-26 16:01:01 UTC) #5
girard
https://codereview.chromium.org/19805005/diff/23002/ui/base/touch/touch_enabled.h File ui/base/touch/touch_enabled.h (right): https://codereview.chromium.org/19805005/diff/23002/ui/base/touch/touch_enabled.h#newcode14 ui/base/touch/touch_enabled.h:14: UI_EXPORT bool IsTouchEnabled(); On 2013/07/26 16:01:01, flackr wrote: > ...
7 years, 4 months ago (2013-07-29 15:01:11 UTC) #6
Ben Goodger (Google)
lgtm
7 years, 4 months ago (2013-07-29 16:55:49 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/girard@chromium.org/19805005/34001
7 years, 4 months ago (2013-07-29 17:15:36 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/girard@chromium.org/19805005/34001
7 years, 4 months ago (2013-07-30 01:32:08 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/girard@chromium.org/19805005/34001
7 years, 4 months ago (2013-07-30 12:07:39 UTC) #10
commit-bot: I haz the power
7 years, 4 months ago (2013-07-31 03:15:59 UTC) #11
Message was sent while issue was closed. 
Change committed as 214535

Powered by Google App Engine
This is Rietveld 408576698