New issue
Advanced search Search tips

Issue 847236 link

Starred by 3 users
Status: Fixed
Owner:
skobes@chromium.org
Closed: May 2018
Cc:
szager@chromium.org
thestig@chromium.org
bokan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 823365



Sign in to add a comment

scrollbar tickmarks not shown on PDF find-in-page

Project Member Reported by skobes@chromium.org, May 28 2018 
1. Open a PDF document, e.g. https://www.irs.gov/pub/irs-pdf/i1040gi.pdf
2. Ctrl-F, type a common word
3. Observe scrollbar

EXPECTED: tickmarks in scrollbar
ACTUAL: no tickmarks in scrollbar

This is an RLS (M66) regression.

Blink appears to have a special hook for plugins to specify tickmarks (WebLocalFrameImpl::SetTickmarks).  This stores the desired tickmarks in LocalFrameView::tickmarks_, which PLSA::GetTickmarks ignores.

Comment 1 by skobes@chromium.org, May 28 2018

pdf-find-no-rls.png
139 KB View Download
pdf-find-rls.png
139 KB View Download

Comment 2 by skobes@chromium.org, May 28 2018 

I discovered this because WebFrameTest has hacky tests for this that directly call LocalFrameView::CreateScrollbar (which we want to remove).

Removing LocalFrameView::CreateScrollbar will require fixing those tests, preferably by fixing this bug.

Comment 3 by e...@chromium.org, May 29 2018

Status: Available (was: Unconfirmed)

Comment 4 by thestig@chromium.org, May 29 2018

Components: UI>Browser>FindInPage
For those of us not in Blink-Land, RLS is Root Layer Scrolls, and PLSA is PaintLayerScrollableArea.

Comment 5 by skobes@chromium.org, May 29 2018

Owner: skobes@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, May 30 2018 

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

commit c6e761442e0c2a98879393aa0f0fec8aefdbb096
Author: Steve Kobes <skobes@chromium.org>
Date: Wed May 30 00:38:27 2018

Fix PDF find-in-page tickmarks.

This moves tickmark overriding and invalidation from LocalFrameView to
LayoutView, and connects PaintLayerScrollableArea correctly.

Bug:  847236 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6b5d5d5cb94a5fb09a67553ab147245e0ca80843
Reviewed-on: https://chromium-review.googlesource.com/1077370
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562674}
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/editing/finder/text_finder.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/editing/finder/text_finder.h
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/editing/markers/document_marker_controller.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/frame/find_in_page.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/frame/local_frame_view.h
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/layout/layout_view.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/layout/layout_view.h
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/c6e761442e0c2a98879393aa0f0fec8aefdbb096/third_party/blink/renderer/core/testing/internals.cc

Comment 7 by skobes@chromium.org, May 30 2018

Status: Fixed (was: Started)

Comment 8 by thestig@chromium.org, Jul 16

Cc: thestig@chromium.org
 Issue 859907  has been merged into this issue.
►

Sign in to add a comment