New issue
Advanced search Search tips

Issue 874132 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Mac touchpad smart zoom is not suppressed in app windows

Project Member Reported by mcnee@chromium.org, Aug 14

Issue description

Chrome Version: 70.0.3522.0
OS: Mac

We currently suppress gesture pinch events for app windows ( see  issue 232098  ). However, we're letting "smart zoom" (double-tap with two fingers on Mac touchpad) events through which also cause page scale changes. We should be consistent here.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86

commit be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu Aug 23 15:10:26 2018

Consistent touchpad pinch behaviour for app windows and webviews

Currently, app windows (possibly containing webviews):
1) suppress all pinch events which also suppresses the
   synthetic wheel event, but not touchscreen touch events, and
2) allow "smart zoom" on Mac which can change the page scale even
   though other means of changing the page scale are suppressed.

As a result of (1), if we have, say, google maps in a webview, a
user can pinch zoom with a touchscreen, but not with a touchpad.

We now suppress gesture events to app windows based on whether they
cause a scale change. So now touchscreen and touchpad pinching are
consistent since the page has access to the events so that it may
implement custom pinch zoom behaviour, but unhandled events still
do not result in a scale change for the app window. Also, smart
zoom is suppressed to be consistent with other touchpad pinching.

Bug:  725970 ,  874132 
Change-Id: I03dd2048002d69dc7c8a822fc727140c67d64706
Reviewed-on: https://chromium-review.googlesource.com/1174933
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585489}
[modify] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/browser/apps/platform_apps/app_browsertest.cc
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/touchpad_pinch/background.js
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/touchpad_pinch/manifest.json
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.html
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.js
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.html
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.js
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.html
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.js
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/manifest.json
[add] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/test.js
[modify] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86/extensions/browser/app_window/app_web_contents_helper.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8555b051b42e9df3700f80d3c5c2530ab3c8f924

commit 8555b051b42e9df3700f80d3c5c2530ab3c8f924
Author: Scott Little <sclittle@chromium.org>
Date: Thu Aug 23 20:58:42 2018

Revert "Consistent touchpad pinch behaviour for app windows and webviews"

This reverts commit be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86.

Reason for revert: Looks like this caused a test to start flaking, https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/12367

Original change's description:
> Consistent touchpad pinch behaviour for app windows and webviews
> 
> Currently, app windows (possibly containing webviews):
> 1) suppress all pinch events which also suppresses the
>    synthetic wheel event, but not touchscreen touch events, and
> 2) allow "smart zoom" on Mac which can change the page scale even
>    though other means of changing the page scale are suppressed.
> 
> As a result of (1), if we have, say, google maps in a webview, a
> user can pinch zoom with a touchscreen, but not with a touchpad.
> 
> We now suppress gesture events to app windows based on whether they
> cause a scale change. So now touchscreen and touchpad pinching are
> consistent since the page has access to the events so that it may
> implement custom pinch zoom behaviour, but unhandled events still
> do not result in a scale change for the app window. Also, smart
> zoom is suppressed to be consistent with other touchpad pinching.
> 
> Bug:  725970 ,  874132 
> Change-Id: I03dd2048002d69dc7c8a822fc727140c67d64706
> Reviewed-on: https://chromium-review.googlesource.com/1174933
> Reviewed-by: Ben Wells <benwells@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Commit-Queue: Kevin McNee <mcnee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585489}

TBR=benwells@chromium.org,ekaramad@chromium.org,wjmaclean@chromium.org,mcnee@chromium.org

Change-Id: I3a717517c60bbe22bbb3b93c64bea107ef191a5a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  725970 ,  874132 
Reviewed-on: https://chromium-review.googlesource.com/1187167
Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585611}
[modify] https://crrev.com/8555b051b42e9df3700f80d3c5c2530ab3c8f924/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/8555b051b42e9df3700f80d3c5c2530ab3c8f924/chrome/browser/apps/platform_apps/app_browsertest.cc
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/touchpad_pinch/background.js
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/touchpad_pinch/manifest.json
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.html
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.js
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.html
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.js
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.html
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.js
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/manifest.json
[delete] https://crrev.com/529b158da2b374d372bdf69b42d06910585c19f4/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/test.js
[modify] https://crrev.com/8555b051b42e9df3700f80d3c5c2530ab3c8f924/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/8555b051b42e9df3700f80d3c5c2530ab3c8f924/extensions/browser/app_window/app_web_contents_helper.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7610825f03ca092acbf312b476a60c167671d97

commit f7610825f03ca092acbf312b476a60c167671d97
Author: Kevin McNee <mcnee@chromium.org>
Date: Fri Aug 24 15:52:47 2018

Reland "Consistent touchpad pinch behaviour for app windows and webviews"

This is a reland of be51aadb8aa5b5c7ee900c3c2721e69ec6b2ea86

Original change's description:
> Consistent touchpad pinch behaviour for app windows and webviews
> 
> Currently, app windows (possibly containing webviews):
> 1) suppress all pinch events which also suppresses the
>    synthetic wheel event, but not touchscreen touch events, and
> 2) allow "smart zoom" on Mac which can change the page scale even
>    though other means of changing the page scale are suppressed.
> 
> As a result of (1), if we have, say, google maps in a webview, a
> user can pinch zoom with a touchscreen, but not with a touchpad.
> 
> We now suppress gesture events to app windows based on whether they
> cause a scale change. So now touchscreen and touchpad pinching are
> consistent since the page has access to the events so that it may
> implement custom pinch zoom behaviour, but unhandled events still
> do not result in a scale change for the app window. Also, smart
> zoom is suppressed to be consistent with other touchpad pinching.
> 
> Bug:  725970 ,  874132 
> Change-Id: I03dd2048002d69dc7c8a822fc727140c67d64706
> Reviewed-on: https://chromium-review.googlesource.com/1174933
> Reviewed-by: Ben Wells <benwells@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Commit-Queue: Kevin McNee <mcnee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585489}

Bug:  725970 ,  874132 
Change-Id: I23420821c1166f7b641faf672798ddac0e928ad9
Reviewed-on: https://chromium-review.googlesource.com/1187345
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585856}
[modify] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/browser/apps/platform_apps/app_browsertest.cc
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/touchpad_pinch/background.js
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/touchpad_pinch/manifest.json
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.html
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/touchpad_pinch/window.js
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.html
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/embedder.js
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.html
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/guest.js
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/manifest.json
[add] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/chrome/test/data/extensions/platform_apps/web_view/touchpad_pinch/test.js
[modify] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/f7610825f03ca092acbf312b476a60c167671d97/extensions/browser/app_window/app_web_contents_helper.cc

Status: Fixed (was: Assigned)

Sign in to add a comment