Prevent ctrl+2f scroll on the touchpad to zoom the page

Prevent ctrl+2f scroll on the touchpad to zoom the page

The CL prevents the crel+2f scroll gesture on the touchpad to zoom the
webpage as we receive complaint of the feature from users. It seems like
an obscure feature that confuses people who do activate it and it is a
little painful to recover from its effect. Ctrl+mousewheel zooming is
not affected by the CL.

Contributed by sheckylin@chromium.org

BUG=323164
TEST=Make sure webpage don't zoom in/out with ctrl+2f scroll on the
touchpad. Additionally, make sure ctrl+alt+2f scroll still works when
the screen magnifier is active.

[Shecky Lin, 2013-12-26 05:02:59]

Hey guys, ben@ is the owner but the issue is gesture related so I added sadrul@
as well. derat@ is the original issue filer. Anyway, please help take a look of
this after you come back to work. Thanks!

[Daniel Erat, 2013-12-26 14:44:22]

Thanks! LGTM (but please wait for Sadrul to take a look to see if there's a
better way of doing this).

https://codereview.chromium.org/112293004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/112293004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_aura.cc:2874: //
WebContentsImpl to zoom the page while retain the scroll event and other
nit: s/to zoom/from zooming/, s/while retain/while retaining/

[Shecky Lin, 2013-12-27 03:23:53]

Thanks!

https://codereview.chromium.org/112293004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/112293004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_aura.cc:2874: //
WebContentsImpl to zoom the page while retain the scroll event and other
On 2013/12/26 14:44:22, Daniel Erat wrote:
> nit: s/to zoom/from zooming/, s/while retain/while retaining/

Done.

[sadrul, 2014-01-03 18:09:16]

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2878:
mouse_wheel_event.modifiers = 0;
If we want to disable this only on chromeos, then this is a reasonable fix
(since we create ui::ScrollEvent only on chromeos). But if we want to do this on
other platforms, then this change should be elsewhere. I have left a comment on
the bug asking about this.

[Rick Byers, 2014-04-04 17:35:53]

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2878:
mouse_wheel_event.modifiers = 0;
On 2014/01/03 18:09:17, sadrul wrote:
> If we want to disable this only on chromeos, then this is a reasonable fix
> (since we create ui::ScrollEvent only on chromeos). But if we want to do this
on
> other platforms, then this change should be elsewhere. I have left a comment
on
> the bug asking about this.

I think it's fine to do this for ChromeOS only for now.  The problem is really
specific to each OS (each have a different way of communicating this and
different sensitivity etc.).  I suggest we land it this way for now, and I can
tweak the Mac and Windows behavior as necessary as part of
http://crbug.com/289887.

[sadrul, 2014-04-04 19:00:20]

Sounds good. Can you add a RenderWidgetHostViewAuraTest for this?

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2878:
mouse_wheel_event.modifiers = 0;
On 2014/04/04 17:35:53, Rick Byers wrote:
> On 2014/01/03 18:09:17, sadrul wrote:
> > If we want to disable this only on chromeos, then this is a reasonable fix
> > (since we create ui::ScrollEvent only on chromeos). But if we want to do
this
> on
> > other platforms, then this change should be elsewhere. I have left a comment
> on
> > the bug asking about this.
> 
> I think it's fine to do this for ChromeOS only for now.  The problem is really
> specific to each OS (each have a different way of communicating this and
> different sensitivity etc.).  I suggest we land it this way for now, and I can
> tweak the Mac and Windows behavior as necessary as part of
> http://crbug.com/289887.
> 

Note that shortly after this patchset, we started generating scroll events on
Windows too (https://codereview.chromium.org/133053004/).

[Shecky Lin, 2014-04-07 08:25:11]

Hey guys, I am busy for some other things so I can't work on it right now. You
may just take the patch and add whatever you want if you are in a hurry. Or, you
may wait a few days and I can take care of it. Sorry about that!

On 2014/04/04 19:00:20, sadrul wrote:
> Sounds good. Can you add a RenderWidgetHostViewAuraTest for this?
> 
>
https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
> File content/browser/renderer_host/render_widget_host_view_aura.cc (right):
> 
>
https://codereview.chromium.org/112293004/diff/60001/content/browser/renderer...
> content/browser/renderer_host/render_widget_host_view_aura.cc:2878:
> mouse_wheel_event.modifiers = 0;
> On 2014/04/04 17:35:53, Rick Byers wrote:
> > On 2014/01/03 18:09:17, sadrul wrote:
> > > If we want to disable this only on chromeos, then this is a reasonable fix
> > > (since we create ui::ScrollEvent only on chromeos). But if we want to do
> this
> > on
> > > other platforms, then this change should be elsewhere. I have left a
comment
> > on
> > > the bug asking about this.
> > 
> > I think it's fine to do this for ChromeOS only for now.  The problem is
really
> > specific to each OS (each have a different way of communicating this and
> > different sensitivity etc.).  I suggest we land it this way for now, and I
can
> > tweak the Mac and Windows behavior as necessary as part of
> > http://crbug.com/289887.
> > 
> 
> Note that shortly after this patchset, we started generating scroll events on
> Windows too (https://codereview.chromium.org/133053004/).

[Daniel Erat, 2014-04-20 13:58:05]

On 2014/04/07 08:25:11, Shecky Lin wrote:
> Hey guys, I am busy for some other things so I can't work on it right now. You
> may just take the patch and add whatever you want if you are in a hurry. Or,
you
> may wait a few days and I can take care of it. Sorry about that!

Do you still need someone to take this patch over?

[Shecky Lin, 2014-04-21 03:51:36]

On 2014/04/20 13:58:05, Daniel Erat wrote:
> On 2014/04/07 08:25:11, Shecky Lin wrote:
> > Hey guys, I am busy for some other things so I can't work on it right now.
You
> > may just take the patch and add whatever you want if you are in a hurry. Or,
> you
> > may wait a few days and I can take care of it. Sorry about that!
> 
> Do you still need someone to take this patch over?

Yep, sorry. I'd be grateful if you guys can take it over. Thanks.