New issue
Advanced search Search tips

Issue 844534 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 19
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 798719



Sign in to add a comment

Implicit Root Scroller doesn't take paint order into account correctly.

Project Member Reported by bokan@chromium.org, May 18 2018

Issue description

As mentioned by chrishtr@ in [1], we currently compare z-index of each implicit candidate to determine which one paints on top. This won't work if they're in different stacking contexts.

The suggested solution from the review:

  "You will have to do a paint-order traversal of the PaintLayer tree to determine which
   scroller paints last. This isn't too hard to implement and should not be all that expensive.

   You just need to do a traversal similar to that in PaintLayer::HitTestChildren,
   stopping at the first PaintLayer found which is an implicit candidate."



[1] https://chromium-review.googlesource.com/c/chromium/src/+/1038648/17/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc#418
 

Comment 1 by bokan@chromium.org, May 18 2018

Blocking: 798719
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 19

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

commit bd27a72858971d401982a7b4f8f83dd91128c556
Author: David Bokan <bokan@chromium.org>
Date: Fri Oct 19 15:32:14 2018

Don't promote implicit root scroller with multiple matches

chrishtr@ pointed out in https://crrev.com/c/1038648 that the paint
order selection used when multiple elements on a page meet all the
criteria of an implicit root scroller isn't correct.

On reflection, it's not immediately clear to me that this is at all
useful so, rather than adding complexity, this CL removes the ordering
and just avoids making any decision at all in this case. We can revisit
if the need comes up in real usage.

Bug:  844534 
Change-Id: I4e05da981eace4062f3898a3a7fe1d4cffdec658
Reviewed-on: https://chromium-review.googlesource.com/c/1288905
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601161}
[modify] https://crrev.com/bd27a72858971d401982a7b4f8f83dd91128c556/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc
[modify] https://crrev.com/bd27a72858971d401982a7b4f8f83dd91128c556/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment