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

Issue 603746 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

accessibility snapshot does not generate correct data on scroll and zoom

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

Issue description

It seems that scroll and offset calculation is broken. This seems to be existent by some cases earlier but fix for crbug/602760 made it more obvious.


 

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

more details are in internal bug b/28186898 but the issue seems like when chrome is scrolled the boundaries were miscalculated. The issue is visible on both x and y scroll. Zoom seems to be ok. Android webview scroll is fine too.

reason:
the snapshot information is collected the same way for webview and chrome, however for some mysterious reason scroll was not behaving same so we were adding extra y and x offsets to our calculations. this calculation was broken.

It turned out that the reason for behavior difference is platform. since webview is a view, platform was calculating the scroll for webview and adding to the view structure. see below, the first child is created by platform, with the scroll parameter.


04-20 21:44:51.031  5903  5903 I AssistStructure:                   View [0,126 1080x1539] android.webkit.WebView
04-20 21:44:51.031  5903  5903 I AssistStructure:                     ID: #ffffffff
04-20 21:44:51.031  5903  5903 I AssistStructure:                     Scroll: 0,316
04-20 21:44:51.031  5903  5903 I AssistStructure:                     Alpha: 0.0
04-20 21:44:51.031  5903  5903 I AssistStructure:                     Children:
04-20 21:44:51.031  5903  5903 I AssistStructure:                       View [0,-316 1605x14644] android.webkit.WebView
04-20 21:44:51.032  5903  5903 I AssistStructure:                         ID: #ffffffff
04-20 21:44:51.032  5903  5903 I AssistStructure:                         Alpha: 0.0
04-20 21:44:51.032  5903  5903 I AssistStructure:                         Text (sel -1--1): 
04-20 21:44:51.032  5903  5903 I AssistStructure:                         Text size: 16.0 , style: #0
04-20 21:44:51.032  5903  5903 I AssistStructure:                         Text color fg: #ff000000, bg: #0
04-20 21:44:51.032  5903  5903 I AssistStructure:                         Children:


so the fix is to ignore offsetting the scroll parameter for webview for the root node when calculating the snapshot tree in chromium but add it for chrome.
Project Member

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

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

commit 98495a3d4de6f2be14a71f675212552b63c71def
Author: sgurun <sgurun@chromium.org>
Date: Sat Apr 23 07:46:23 2016

Fix a nasty scroll bug for Chrome Now-on-tap feature. Also combine the
code paths for Android Webview and Chrome.

Until now webview and chrome were using separate code paths for copying the snapshot tree. We are now combining both to use the same code. Further moving the
tests from android webview to content layer to add coverage for both chrome
and webview.

BUG= 603746 

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

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

[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/BUILD.gn
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content_public/browser/AccessibilitySnapshotNode.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java
[add] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/javatests/src/org/chromium/content/browser/webcontents/AccessibilitySnapshotTest.java

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 25 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/b34b3bf5fddbca06401a16396505d6bcec454849

commit b34b3bf5fddbca06401a16396505d6bcec454849
Author: sgurun <sgurun@chromium.org>
Date: Sat Apr 23 07:46:23 2016

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 25 2016

Labels: merge-merged-2716
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98495a3d4de6f2be14a71f675212552b63c71def

commit 98495a3d4de6f2be14a71f675212552b63c71def
Author: sgurun <sgurun@chromium.org>
Date: Sat Apr 23 07:46:23 2016

Fix a nasty scroll bug for Chrome Now-on-tap feature. Also combine the
code paths for Android Webview and Chrome.

Until now webview and chrome were using separate code paths for copying the snapshot tree. We are now combining both to use the same code. Further moving the
tests from android webview to content layer to add coverage for both chrome
and webview.

BUG= 603746 

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

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

[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/BUILD.gn
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content_public/browser/AccessibilitySnapshotNode.java
[modify] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java
[add] https://crrev.com/98495a3d4de6f2be14a71f675212552b63c71def/content/public/android/javatests/src/org/chromium/content/browser/webcontents/AccessibilitySnapshotTest.java

Comment 5 by sgu...@chromium.org, Apr 26 2016

Labels: Merge-Request-51

Comment 6 by tin...@google.com, Apr 26 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 7 by bugdroid1@chromium.org, Apr 26 2016

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

commit d3859f059d3cc32a156a830d0de3080f3cfe7b48
Author: Selim Gurun <sgurun@google.com>
Date: Tue Apr 26 18:45:11 2016

Fix a nasty scroll bug for Chrome Now-on-tap feature. Also combine the code paths for Android Webview and Chrome.

Until now webview and chrome were using separate code paths for copying the snapshot tree. We are now combining both to use the same code. Further moving the
tests from android webview to content layer to add coverage for both chrome
and webview.

BUG= 603746 

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

Cr-Commit-Position: refs/heads/master@{#389374}
(cherry picked from commit 98495a3d4de6f2be14a71f675212552b63c71def)

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

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

[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/BUILD.gn
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content_public/browser/AccessibilitySnapshotNode.java
[modify] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java
[add] https://crrev.com/d3859f059d3cc32a156a830d0de3080f3cfe7b48/content/public/android/javatests/src/org/chromium/content/browser/webcontents/AccessibilitySnapshotTest.java

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/8249ee454ad2140b256758d904ea262ef3fe9c34

commit 8249ee454ad2140b256758d904ea262ef3fe9c34
Author: sgurun <sgurun@chromium.org>
Date: Sat Apr 23 07:46:23 2016

Comment 9 by sgu...@chromium.org, Apr 26 2016

Status: Fixed (was: Assigned)

Sign in to add a comment