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

Issue 165143003: Handle Ctrl+Mousewheel after renderer (Closed)

Created:
6 years, 10 months ago by robwu
Modified:
6 years, 9 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Handle Ctrl+Mousewheel after renderer Moves handling of Ctrl+Mousewheel for zoom after the renderer. This allows web pages to observe and intercept the Ctrl + Mousewheel event. Depends on a patch in Blink: https://codereview.chromium.org/168493002 Contributed by Rob Wu <rob@robwu.nl>; BUG=111059 TEST=Manual; Open the HTML file from https://code.google.com/p/chromium/issues/detail?id=111059#c15 and follow the instructions

Patch Set 1 #

Patch Set 2 : Fix ctrl+wheel when scrollbars are present #

Total comments: 1

Patch Set 3 : rebase #

Patch Set 4 : revert unnecessary change #

Patch Set 5 : "unnecessary change" turned out to be necessary #

Patch Set 6 : Disable Ctrl+wheel checks on Mac #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -12 lines) Patch
M content/browser/renderer_host/input/input_router_impl.cc View 1 2 3 4 5 2 chunks +14 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_delegate.h View 1 chunk +4 lines, -3 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 2 chunks +6 lines, -5 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M content/renderer/input/input_handler_proxy.cc View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
robwu
jam: Please review. raymes: FYI
6 years, 10 months ago (2014-02-14 15:55:11 UTC) #1
robwu
Hm, I just noticed an issue with this patch: When the page has scrollbars, Ctrl ...
6 years, 10 months ago (2014-02-14 16:09:19 UTC) #2
robwu
Fixed the scrollwheel issue. Depends on a patch in Blink: https://codereview.chromium.org/168493002/
6 years, 10 months ago (2014-02-15 10:57:45 UTC) #3
raymes
On 2014/02/15 10:57:45, robwu wrote: > Fixed the scrollwheel issue. Depends on a patch in ...
6 years, 10 months ago (2014-02-16 23:06:57 UTC) #4
robwu
On 2014/02/16 23:06:57, raymes wrote: > On 2014/02/15 10:57:45, robwu wrote: > > Fixed the ...
6 years, 10 months ago (2014-02-17 00:10:30 UTC) #5
robwu
My patch for Blink was accepted and included in the Blink roll of today. jam: ...
6 years, 10 months ago (2014-02-20 17:01:23 UTC) #6
raymes
On 2014/02/20 17:01:23, robwu wrote: > My patch for Blink was accepted and included in ...
6 years, 10 months ago (2014-02-21 03:24:42 UTC) #7
jam
+jdduke for review of input files Has anyone run this with UI leads to ensure ...
6 years, 10 months ago (2014-02-26 00:02:37 UTC) #8
jdduke (slow)
I'd also like to hear what the UI folks say before I make any detailed ...
6 years, 10 months ago (2014-02-26 00:24:39 UTC) #9
raymes
Just for the record, ctrl++ and ctrl+- can be overridden. On Wed, Feb 26, 2014 ...
6 years, 10 months ago (2014-02-26 00:27:13 UTC) #10
jamesr
It seems like this is mostly a UX question of whether the page should be ...
6 years, 10 months ago (2014-02-26 23:51:51 UTC) #11
robwu
On 2014/02/26 23:51:51, jamesr wrote: > It seems like this is mostly a UX question ...
6 years, 9 months ago (2014-02-27 23:03:08 UTC) #12
jdduke (slow)
On 2014/02/27 23:03:08, robwu wrote: > I have amended the CL to revert this change, ...
6 years, 9 months ago (2014-02-27 23:36:54 UTC) #13
robwu
On 2014/02/27 23:36:54, jdduke wrote: > On 2014/02/27 23:03:08, robwu wrote: > > I have ...
6 years, 9 months ago (2014-02-28 11:26:06 UTC) #14
jdduke (slow)
On 2014/02/28 11:26:06, robwu wrote: > Apologies, I have made an error in my previous ...
6 years, 9 months ago (2014-02-28 14:18:53 UTC) #15
robwu
On 2014/02/28 14:18:53, jdduke wrote: > On 2014/02/28 11:26:06, robwu wrote: > > Apologies, I ...
6 years, 9 months ago (2014-02-28 17:34:51 UTC) #16
jdduke (slow)
Per recent discussion with rbyers@ I'll defer to his thoughts on this change.
6 years, 9 months ago (2014-03-04 18:55:07 UTC) #17
Rick Byers
On 2014/03/04 18:55:07, jdduke wrote: > Per recent discussion with rbyers@ I'll defer to his ...
6 years, 9 months ago (2014-03-04 19:25:12 UTC) #18
robwu
On 2014/03/04 19:25:12, Rick Byers wrote: > On 2014/03/04 18:55:07, jdduke wrote: > > Per ...
6 years, 9 months ago (2014-03-04 23:09:41 UTC) #19
Rick Byers
6 years, 9 months ago (2014-03-04 23:30:43 UTC) #20
Message was sent while issue was closed.
On 2014/03/04 23:09:41, robwu wrote:
> On 2014/03/04 19:25:12, Rick Byers wrote:
> > On 2014/03/04 18:55:07, jdduke wrote:
> > > Per recent discussion with rbyers@ I'll defer to his thoughts on this
> change.
> > 
> > Wow, I had no idea this was going in parallel - I'm sorry!  See this
> > chromium-dev thread from awhile back about this:
> >
>
https://groups.google.com/a/chromium.org/forum/#%2521searchin/chromium-dev/mo...,
> > and this CL I just landed: https://codereview.chromium.org/177213016/.  I'll
> > dupe the bugs together somehow and we can discuss further in the bugs.
> 
> No problem, I'm glad that the bug got fixed.
> Note that the Blink counterpart of this CL
> (https://src.chromium.org/viewvc/blink?revision=167355&view=revision) must not
> be reverted, because your patch also depends on it (via
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host...).

Yep, I noticed that too.  I should have checked when that code was added (I
assumed it had been there for awhile).  But it's covered by the layout test I
added - if someone changes/removed it the test will fail:
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/wheelevent...

> How can I close this CL without deleting it?

Just click the 'x' next to the issue title.

Powered by Google App Engine
This is Rietveld 408576698