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

Issue 810067 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
OOO until Jan 18
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] Android find-in-page broken on desktop pages

Project Member Reported by bokan@chromium.org, Feb 7 2018

Issue description

OS Version: Android

What steps will reproduce the problem?
1. Launch chrome with --root-layer-scrolls
2. Visit a desktop page (e.g. www.reddit.com) with "Desktop Site" from the "three dot menu"
3. Search for something on the page (e.g. "next", "comments")

What is the expected result?
Cycling through results should scroll and zoom them into view

What happens instead of that?
If anything happens at all, we seem to zoom in near the top of the page but not to the where the result is (if it's further down the page).

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.31 Safari/537.36



 

Comment 1 by bokan@chromium.org, Feb 7 2018

Blocking: 417782

Comment 2 by bokan@chromium.org, Feb 7 2018

Seems to work ok on mobile pages - I'm guessing zooming takes a different path.
Owner: bokan@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 13 2018

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

commit e4e1aa3603cd167f2f132936575c09a6a8bedb61
Author: David Bokan <bokan@chromium.org>
Date: Tue Feb 13 15:23:28 2018

Add a scrolling test for find-in-page

Turning on root-layer-scrolling caused find in page to break in Android
but wasn't turned up by tests. This CL adds a test to fill the gap. The
regression was incidentally fixed in r535827.

Also update TextFinder to use AbsoluteToRootFrame rather than
ContentsToRootFrame. The two are equivalent but I'd like to eventually
remove the Contents nomenclature since it can be easily confused for
being relative to the document origin.

This CL has no functional changes.

Bug:  810067 
Change-Id: Idec7335f17e45184467ea14e403504efe7f9771c
Reviewed-on: https://chromium-review.googlesource.com/912510
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Paul Meyer <paulmeyer@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536359}
[modify] https://crrev.com/e4e1aa3603cd167f2f132936575c09a6a8bedb61/content/browser/find_request_manager_browsertest.cc
[add] https://crrev.com/e4e1aa3603cd167f2f132936575c09a6a8bedb61/content/test/data/find_in_page_desktop.html
[modify] https://crrev.com/e4e1aa3603cd167f2f132936575c09a6a8bedb61/third_party/WebKit/Source/core/editing/finder/TextFinder.cpp

Comment 5 by bokan@chromium.org, Feb 13 2018

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 13 2018

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

commit b5a86a42beab584c4bed87cac07b7b6af0ab8798
Author: Marc Treib <treib@chromium.org>
Date: Tue Feb 13 16:07:43 2018

Revert "Add a scrolling test for find-in-page"

This reverts commit e4e1aa3603cd167f2f132936575c09a6a8bedb61.

Reason for revert: Broke the build on WinMSVC64:
https://ci.chromium.org/buildbot/chromium.win/WinMSVC64%20(dbg)/2868

Original change's description:
> Add a scrolling test for find-in-page
> 
> Turning on root-layer-scrolling caused find in page to break in Android
> but wasn't turned up by tests. This CL adds a test to fill the gap. The
> regression was incidentally fixed in r535827.
> 
> Also update TextFinder to use AbsoluteToRootFrame rather than
> ContentsToRootFrame. The two are equivalent but I'd like to eventually
> remove the Contents nomenclature since it can be easily confused for
> being relative to the document origin.
> 
> This CL has no functional changes.
> 
> Bug:  810067 
> Change-Id: Idec7335f17e45184467ea14e403504efe7f9771c
> Reviewed-on: https://chromium-review.googlesource.com/912510
> Reviewed-by: Steve Kobes <skobes@chromium.org>
> Reviewed-by: Paul Meyer <paulmeyer@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536359}

TBR=bokan@chromium.org,skobes@chromium.org,paulmeyer@chromium.org

Change-Id: Ia7ce410cacab878bb05820461e485cdc588c7209
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  810067 
Reviewed-on: https://chromium-review.googlesource.com/916481
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536365}
[modify] https://crrev.com/b5a86a42beab584c4bed87cac07b7b6af0ab8798/content/browser/find_request_manager_browsertest.cc
[delete] https://crrev.com/9de932d85d1909b8f2d1fd2cf7276980f0d1f586/content/test/data/find_in_page_desktop.html
[modify] https://crrev.com/b5a86a42beab584c4bed87cac07b7b6af0ab8798/third_party/WebKit/Source/core/editing/finder/TextFinder.cpp

Comment 7 by bokan@chromium.org, Feb 13 2018

Status: Started (was: Fixed)
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 13 2018

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

commit 7ebbe90a16083547e3af395ba694c88196982b9a
Author: David Bokan <bokan@chromium.org>
Date: Tue Feb 13 17:41:54 2018

[Reland] Add a scrolling test for find-in-page

Turning on root-layer-scrolling caused find in page to break in Android
but wasn't turned up by tests. This CL adds a test to fill the gap. The
regression was incidentally fixed in r535827.

Also update TextFinder to use AbsoluteToRootFrame rather than
ContentsToRootFrame. The two are equivalent but I'd like to eventually
remove the Contents nomenclature since it can be easily confused for
being relative to the document origin.

This CL has no functional changes.

[Reland Note] Fixes unreachable code build failure.

TBR=treib@chromium.org,skobes@chromium.org,paulmeyer@chromium.org

Bug:  810067 
Change-Id: Iee35b2e5ef27c4a8985080b6962108f64b947a17
Reviewed-on: https://chromium-review.googlesource.com/915786
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536403}
[modify] https://crrev.com/7ebbe90a16083547e3af395ba694c88196982b9a/content/browser/find_request_manager_browsertest.cc
[add] https://crrev.com/7ebbe90a16083547e3af395ba694c88196982b9a/content/test/data/find_in_page_desktop.html
[modify] https://crrev.com/7ebbe90a16083547e3af395ba694c88196982b9a/third_party/WebKit/Source/core/editing/finder/TextFinder.cpp

Comment 9 by bokan@chromium.org, Feb 13 2018

Status: Fixed (was: Started)
Hopefully will stick this time.

Sign in to add a comment