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

Issue 177213016: Give blink a chance to consume ctrl+wheel events before zooming (Closed)

Created:
6 years, 10 months ago by Rick Byers
Modified:
6 years, 9 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, miu+watch_chromium.org
Visibility:
Public.

Description

Give blink a chance to consume ctrl+wheel events before zooming Today chromium consumes all mouse wheel events when the control key is pressed for the purposes of browser zoom (except on MacOS). With this change we give the web page a chance to get and consume the wheel event first, in case it wants to override the behavior. This makes Chrome consistent with IE and Firefox on Windows. See http://crbug.com/289887 and the linked chromium-dev thread for discussion. Apparently there were no tests at all for this functionality. I've added new unit tests at RenderWidgetHost and WebContents layers. Depends on blink change http://src.chromium.org/viewvc/blink?view=rev&rev=168088 BUG=289887 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=254559

Patch Set 1 #

Patch Set 2 : Add tests #

Total comments: 4

Patch Set 3 : Tweaks for sadrul CR #

Patch Set 4 : Merge with trunk (no changes) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+178 lines, -20 lines) Patch
M content/browser/renderer_host/render_widget_host_delegate.h View 1 chunk +4 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 2 3 2 chunks +4 lines, -5 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_unittest.cc View 1 2 3 9 chunks +50 lines, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 2 chunks +1 line, -2 lines 0 comments Download
M content/browser/web_contents/web_contents_impl_unittest.cc View 1 2 3 3 chunks +91 lines, -0 lines 0 comments Download
M content/renderer/input/input_handler_proxy.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M content/renderer/input/input_handler_proxy_unittest.cc View 1 7 chunks +21 lines, -6 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Rick Byers
Sadrul, please review.
6 years, 9 months ago (2014-02-28 01:19:06 UTC) #1
sadrul
LGTM https://codereview.chromium.org/177213016/diff/20001/content/browser/web_contents/web_contents_impl_unittest.cc File content/browser/web_contents/web_contents_impl_unittest.cc (right): https://codereview.chromium.org/177213016/diff/20001/content/browser/web_contents/web_contents_impl_unittest.cc#newcode2239 content/browser/web_contents/web_contents_impl_unittest.cc:2239: last_zoom_in_(false) brace here https://codereview.chromium.org/177213016/diff/20001/content/browser/web_contents/web_contents_impl_unittest.cc#newcode2261 content/browser/web_contents/web_contents_impl_unittest.cc:2261: bool last_zoom_in_; DISALLOW_COPY_..
6 years, 9 months ago (2014-03-03 12:37:56 UTC) #2
Rick Byers
Thanks Sadrul. +avi for OWNERS. https://codereview.chromium.org/177213016/diff/20001/content/browser/web_contents/web_contents_impl_unittest.cc File content/browser/web_contents/web_contents_impl_unittest.cc (right): https://codereview.chromium.org/177213016/diff/20001/content/browser/web_contents/web_contents_impl_unittest.cc#newcode2239 content/browser/web_contents/web_contents_impl_unittest.cc:2239: last_zoom_in_(false) On 2014/03/03 12:37:56, ...
6 years, 9 months ago (2014-03-03 14:52:52 UTC) #3
Avi (use Gerrit)
lgtm stampity stamp
6 years, 9 months ago (2014-03-03 15:59:06 UTC) #4
Rick Byers
The CQ bit was checked by rbyers@chromium.org
6 years, 9 months ago (2014-03-03 16:02:53 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rbyers@chromium.org/177213016/60001
6 years, 9 months ago (2014-03-03 16:03:15 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-03 17:17:53 UTC) #7
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=273067
6 years, 9 months ago (2014-03-03 17:17:53 UTC) #8
Rick Byers
The CQ bit was checked by rbyers@chromium.org
6 years, 9 months ago (2014-03-03 17:34:57 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rbyers@chromium.org/177213016/60001
6 years, 9 months ago (2014-03-03 17:39:32 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rbyers@chromium.org/177213016/60001
6 years, 9 months ago (2014-03-03 18:29:03 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rbyers@chromium.org/177213016/60001
6 years, 9 months ago (2014-03-03 19:55:39 UTC) #12
commit-bot: I haz the power
6 years, 9 months ago (2014-03-03 21:04:16 UTC) #13
Message was sent while issue was closed.
Change committed as 254559

Powered by Google App Engine
This is Rietveld 408576698