New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 602760 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2016
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Now on tap bug: the coordinate system is broken for Chrome on Zoom

Project Member Reported by sgu...@chromium.org, Apr 12 2016

Issue description

the boundaries in chrome is incorrectly reported when doing zoom. There are a few examples in internal link b/28037532 and b/27442158 but not publishing here.

This seems to be a regression since it used to work ok. The regression impacts scroll and zoom.
 

Comment 1 by sgu...@chromium.org, Apr 12 2016

This is a chrome bug, webview works ok
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 13 2016

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

commit 93c4e8bb9c0418a3822d30428cd6c2d59eff0a40
Author: sgurun <sgurun@chromium.org>
Date: Wed Apr 13 00:30:14 2016

Fix the scroll and zoom

BUG= 602760 

In a previous code change, the scroll and zoom parameters were placed out
of their place. Unfortunately this caused the coordinates to be be wrong
when scroll and zoom is done. This CL fixes it.

Writing an integration test seemed more complicated then it is. The most
suitable test location to confirm the parameters is
onProvideVirtualStructure() method right before creating the Android
ViewStructure. However, this structure does not seem to be mockable.

I will look for a better testing (or perhaps refactoring the code)
in a next CL. But this CL needs to be mergeable to 51 so not doing it
here.

Review URL: https://codereview.chromium.org/1882283002

Cr-Commit-Position: refs/heads/master@{#386875}

[modify] https://crrev.com/93c4e8bb9c0418a3822d30428cd6c2d59eff0a40/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/93c4e8bb9c0418a3822d30428cd6c2d59eff0a40/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

Comment 3 by sgu...@chromium.org, Apr 13 2016

Labels: Merge-Request-51

Comment 4 by tin...@google.com, Apr 13 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 13 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f93c1da191008f0badde61c140c3765fe14013d6

commit f93c1da191008f0badde61c140c3765fe14013d6
Author: Selim Gurun <sgurun@google.com>
Date: Wed Apr 13 22:44:54 2016

Fix the scroll and zoom

BUG= 602760 

In a previous code change, the scroll and zoom parameters were placed out
of their place. Unfortunately this caused the coordinates to be be wrong
when scroll and zoom is done. This CL fixes it.

Writing an integration test seemed more complicated then it is. The most
suitable test location to confirm the parameters is
onProvideVirtualStructure() method right before creating the Android
ViewStructure. However, this structure does not seem to be mockable.

I will look for a better testing (or perhaps refactoring the code)
in a next CL. But this CL needs to be mergeable to 51 so not doing it
here.

Review URL: https://codereview.chromium.org/1882283002

Cr-Commit-Position: refs/heads/master@{#386875}
(cherry picked from commit 93c4e8bb9c0418a3822d30428cd6c2d59eff0a40)

Review URL: https://codereview.chromium.org/1881343003 .

Cr-Commit-Position: refs/branch-heads/2704@{#39}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/f93c1da191008f0badde61c140c3765fe14013d6/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/f93c1da191008f0badde61c140c3765fe14013d6/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

Comment 6 by sgu...@chromium.org, Apr 13 2016

Status: Fixed (was: Assigned)

Sign in to add a comment