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

Issue 603997 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Incorrect clipping about ancestor clipping layer

Project Member Reported by wangxianzhu@chromium.org, Apr 15 2016

Issue description

Separated from  bug 599682 :

Comment 31 by ericrk@chromium.org, Apr 5, 2016
Delete comment ⚐
Here's a simple repro html.
 	test_scroll.html 
11.6 KB View Download	
Delete

Comment 36 by vollick@chromium.org, Apr 6, 2016
Delete comment ⚐
Yep, we're certainly computing the geometry of the ancestor clipping layer incorrectly.

We set up the arguments to PaintLayerClipper::backgroundClipRect here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp&l=817

In this case we have a composited layer tree that roughly looks like this (note: greatly simplified)
 * composited ancestor
   * scroll clip
     * overflow hidden clip
   * ancestor clipping layer
     * thing that's scrolled, but since the scroller isn't an overflow scroller doesn't force a stacking context.

Normally content with a "scroll parent" doesn't need an ancestor clip layer. The CompositedLayerMapping decides that since its background clip rect is infinite if we ignore the overflow scroll clip, we can just set up the scroll parent pointer and we're happy.

In this case, the overflow hidden layer (as can be seen in the tree) is not an ancestor of the scrolled content, so we need an ACL but the clipper produces the wrong value.

I think the simplest solution is to fix the clipping container we use. In this case, the clipping container should be the scroll parent if we have one. If we do this the clipper can correctly ignore the overflow clip.

I haven't yet tested for unexpected side effects, but the following patch corrects the sizing of the ACL.
https://codereview.chromium.org/1863143002

 
test_scroll (1).html
11.6 KB View Download
Status: Fixed (was: Assigned)
Labels: Hotlist-Threaded-Rendering
Project Member

Comment 5 by bugdroid1@chromium.org, May 5 2016

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

commit efadb1362a06b05bc877a13f50010ddf3ce50ccd
Author: vollick <vollick@chromium.org>
Date: Thu May 05 21:40:02 2016

Revert of Use correct clip container when computing acl for scroll children

Reason for revert:
Caused  issue 608410 

Original issue's description:
> Use correct clip container when computing acl for scroll children
>
> Kudos to erick@ for the reduced test case.
>
> BUG= 603997 
>
> Committed: https://crrev.com/94d5fad34f0351e61fd658ca4ed93648c415620d
> Cr-Commit-Position: refs/heads/master@{#390199}

TBR=wangxianzhu@chromium.org
BUG= 603997 ,  608410 

Review-Url: https://codereview.chromium.org/1951853006
Cr-Commit-Position: refs/heads/master@{#391913}

[modify] https://crrev.com/efadb1362a06b05bc877a13f50010ddf3ce50ccd/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/efadb1362a06b05bc877a13f50010ddf3ce50ccd/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/efadb1362a06b05bc877a13f50010ddf3ce50ccd/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp

Status: Assigned (was: Fixed)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 17 2016

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

commit d1997c29e5846c79df3efd9f383ce340f03f8efb
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Fri Jun 17 20:36:34 2016

Change remaining NeedsManualRebaseline to Failure or Pass Failure

Some tests have been marked NeedsManualRebaseline for a long
time without being rebaselined.

Some tests can't be rebaselined because of flakiness.

BUG=487344, 620126 , 592409 , 603997 , 597221 , 569139 

Review-Url: https://codereview.chromium.org/2075993002
Cr-Commit-Position: refs/heads/master@{#400492}

[modify] https://crrev.com/d1997c29e5846c79df3efd9f383ce340f03f8efb/third_party/WebKit/LayoutTests/TestExpectations

So this patch landed and then was reverted, but the test rebaseline was not reverted. I think I should revert back to the original expectations that match the code as it now is. Any disagreement?
Ok.

Ian, what's the status of this bug? Please unassign if you aren't working on it.
Cc: vollick@chromium.org trchen@chromium.org
Owner: ----
Status: Available (was: Assigned)
I did stop looking at this after my naive fix proved incorrect. Would be great if someone had time to look into this properly.
Labels: -Hotlist-Threaded-Rendering Hotlist-ThreadedRendering
Owner: flackr@chromium.org
Assigning to flackr@ to drive prioritizing of this bug. I'm not even clear from reading the thread what the buggy behavior is, so it's hard for me to prioritize.
Project Member

Comment 14 by sheriffbot@chromium.org, Apr 13 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: WontFix (was: Untriaged)
This seems to be working now. We match Firefox on the test case in the first comment. Any number of things might have fixed it.

Sign in to add a comment