New issue
Advanced search Search tips

Issue 826219 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

Scrollbar indicators for find-in-page hits move around chaotically

Project Member Reported by grt@chromium.org, Mar 27 2018

Issue description

67.0.3378.0 (Official Build) canary (64-bit) (cohort: Clang-64)

Repro: open a Chromium code review, expand all files, C-f and type something that will have some number of hits, then do things like scroll via scrollwheel or C-g to move around the results. Notice that the yellow bars in the scroll area not only don't seem to correspond in any meaningful way with the hits on the page, but they jump around randomly as you scroll.

Pre-emptively adding RBS since this is quite a bad UX.
 

Comment 1 by e...@chromium.org, Mar 28 2018

Cc: bokan@chromium.org skobes@chromium.org szager@chromium.org

Comment 2 by bokan@chromium.org, Mar 28 2018

Labels: Needs-Bisect OS-Linux
Able to repro on Linux. A bisect would be helpful

Comment 3 by bokan@chromium.org, Mar 28 2018

Cc: -bokan@chromium.org
Labels: -Needs-Bisect
Owner: bokan@chromium.org
Status: Assigned (was: Untriaged)
Nevermind - it's RootLayerScrolling :(

I'll take a closer look tomorrow.

Comment 4 by skobes@chromium.org, Mar 28 2018

Blocking: 417782

Comment 5 by bokan@chromium.org, Mar 29 2018

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2018

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

commit 62cc7d83fd0870e628bee9a9fad7a470b77e0e3e
Author: David Bokan <bokan@chromium.org>
Date: Thu Mar 29 23:13:34 2018

[root layer scrolls] Fix find-in-page tickmarks

Tickmarks are calculated based on Document-relative rects but turning on
RLS made these rect Frame-relative since they're "absolute". This patch
explicitly converts them to document coordinates.

Bug:  826219 
Change-Id: Id39f1b337f2bebe34587d79398879778ddb864cc
Reviewed-on: https://chromium-review.googlesource.com/986715
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547008}
[modify] https://crrev.com/62cc7d83fd0870e628bee9a9fad7a470b77e0e3e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/62cc7d83fd0870e628bee9a9fad7a470b77e0e3e/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

Comment 7 by bokan@chromium.org, Mar 29 2018

Will let this roll through canary and request a merge after the weekend.

Comment 8 by bokan@chromium.org, Apr 3 2018

Labels: Merge-Request-66
Confirmed on Windows 67.0.3387.0. Requesting merge.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 3 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 M-66 Merge-Approved-66
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 3 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6213623322eefe6089ed0be0638f393006af4b5b

commit 6213623322eefe6089ed0be0638f393006af4b5b
Author: David Bokan <bokan@chromium.org>
Date: Tue Apr 03 18:29:42 2018

[root layer scrolls] Fix find-in-page tickmarks

Tickmarks are calculated based on Document-relative rects but turning on
RLS made these rect Frame-relative since they're "absolute". This patch
explicitly converts them to document coordinates.

Bug:  826219 
Change-Id: Id39f1b337f2bebe34587d79398879778ddb864cc
Reviewed-on: https://chromium-review.googlesource.com/986715
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#547008}(cherry picked from commit 62cc7d83fd0870e628bee9a9fad7a470b77e0e3e)
Reviewed-on: https://chromium-review.googlesource.com/993312
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#553}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/6213623322eefe6089ed0be0638f393006af4b5b/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/6213623322eefe6089ed0be0638f393006af4b5b/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

Status: Fixed (was: Started)

Sign in to add a comment