My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 246728: CSS Clip Not Working in Fixed Position Elements
8 people starred this issue and may be notified of changes. Back to list
 
Reported by carlan...@google.com, Jun 4, 2013
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.93 Safari/537.36

Example URL:
http://ipomea.mtv.corp.google.com/~carlanton/test/fixed-absolute.html

Steps to reproduce the problem:
ChromeOS only, not Chrome browser.  This problem can be seen when a CSS clip rect is applied to a CSS position:fixed Element that contains a CSS position:absolute element that extends above the fixed element.

What is the expected behavior?
When the test page is properly rendered, you will see a green bar over the tan box.

What went wrong?
In the test page on ChromeOS only (Not Chrome browser), you see instead a red bar over the tan box.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? Yes Previous to recent ChromeOs update

Does this work in other browsers? Yes Chrome Browser Versions 29.0.1528.0 canary, 27.0.1453.93

Chrome version: 28.0.1500.20 beta  Channel: beta
OS Version: 28.0.1500.20

This is causing rendering problems for Gmail, which uses this technique for masking and clipping the recently redesigned Reply.
fixed-absolute.html
2.9 KB   View   Download
clip-correct.png
21.0 KB   View   Download
clip-wrong.png
22.7 KB   View   Download
Jun 4, 2013
#1 carlan...@google.com
I wanted to put this bug under Blink-Rendering/Blink-Compositing, but I don't see a way to do that.
Jun 4, 2013
#2 komoro...@chromium.org
I'm marking this as a release blocker for stable given that it affects a popular website and appears to be fixed, so hopefully once we get a good bisect it will be easy to fix before stable. 

Mike, any idea what this is?
Cc: mikelawt...@chromium.org
Labels: -Type-Bug Type-Bug-Regression M-28 Hotlist-GoogleApps Needs-Bisect ReleaseBlock-Stable
Jun 5, 2013
#3 kar...@google.com
vivian can we have a bisect foor ths? or at least check if it's cros only? if it's blink, it seems unlikely that it should be cros only.
Status: Assigned
Owner: vivi...@chromium.org
Jun 14, 2013
#4 vivi...@chromium.org
Karen, there is no way to bisect bugs on Chrome OS. need a proper owner for this
Status: Untriaged
Owner: ---
Jun 14, 2013
#5 carlan...@google.com
We definitely need this fix for Gmail.

Carl


Jun 17, 2013
#6 vivi...@chromium.org
(No comment was entered for this change.)
Cc: mbo...@chromium.org
Jun 17, 2013
#7 vivi...@chromium.org
Mrudula,

could you check see if this issue can be reproduced on Linux? thanks!
Jun 17, 2013
#8 mbo...@chromium.org
Not reproducible. Green bar over the tan box for Chrome 29.0.1535.3 dev, 28.0.1500.45 beta/stable on Linux Ubuntu 12.04 Precise. 
Screenshot from 2013-06-17 15:12:33.png
70.6 KB   View   Download
Jun 17, 2013
#9 kar...@google.com
i don't see how we can have a blink bug that affects only cros. piman could it be some compositing issue?
Status: Assigned
Owner: piman@chromium.org
Jun 17, 2013
#10 piman@chromium.org
->vangelis to find an owner
Owner: vangelis@chromium.org
Jun 17, 2013
#11 piman@chromium.org
(No comment was entered for this change.)
Cc: voll...@chromium.org shawnsingh@chromium.org
Jun 18, 2013
#12 vangelis@chromium.org
I believe this is a side-effect of us starting to composite fixed position elements (which is on by default on hi-dpi machines such as the retina mac and pixel).  Disabling "Compositing for fixed position elements" in about:flags makes the issue go away. Also, I doesn't appear to be fixed in 29.

Shawn, can you please have a look ?

Owner: shawnsingh@chromium.org
Cc: -shawnsingh@chromium.org vangelis@chromium.org
Jun 18, 2013
#13 shawnsingh@chromium.org
I investigated a bit.  This is not related to fixed-position elements; that's just the compositing trigger.  The bug seems to be for any composited element with a css clip property.  This clip is when trying to compute the main layer's composited bounds.  I think the fix should be placed in RenderLayer::calculateLayerBounds(), but I'm not entirely clear on how that code works, hopefully Julien or Elliot can discuss with me offline soon to get this fixed.

I've attached a test case that shows the problem (webkit nightly and chromium tip of tree), without fixed-pos elements.  Try toggling compositing on the element, and toggling the clip.  The correct behavior is when compositing is off.

Here is hacked code that fixes the problem.  But I think this code is blatantly wrong for other reasons.  It does illustrate that layer bounds are missing the clipRect, however.

In the meantime, if someone needs to workaround this, the solution is to create a separate overflow div that does this clipping instead of the clip property.

diff --git a/Source/core/rendering/RenderLayerBacking.cpp b/Source/core/rendering/RenderLayerBacking.cpp
index a5a84c2..ae44471 100644
--- a/Source/core/rendering/RenderLayerBacking.cpp
+++ b/Source/core/rendering/RenderLayerBacking.cpp
@@ -310,6 +310,10 @@ void RenderLayerBacking::updateCompositedBounds()
 {
     IntRect layerBounds = compositor()->calculateCompositedBounds(m_owningLayer, m_owningLayer);
 
+    // not correct, just a hack...
+    if (!compositor()->clipsCompositingDescendants(m_owningLayer) && m_owningLayer->renderer()->hasClip())
+        layerBounds = pixelSnappedIntRect(toRenderBox(m_owningLayer->renderer())->clipRect(LayoutPoint(), 0));
+
     // Clip to the size of the document or enclosing overflow-scroll layer.
     // If this or an ancestor is transformed, we can't currently compute the correct rect to intersect with.
     // We'd need RenderObject::convertContainerToLocalQuad(), which doesn't yet exist.

css-clip-problem.html
736 bytes   View   Download
Cc: jchaffraix@chromium.org esprehn@chromium.org
Jun 20, 2013
#14 shawnsingh@chromium.org
(No comment was entered for this change.)
Labels: -Pri-2 Pri-1
Jun 20, 2013
#15 shawnsingh@chromium.org
A side comment - according to CSS 2.1 spec, the css clip property only applies to absolutely positioned elements.  I assume this means it should not apply to fixed-position elements.  Maybe there is something updated in css 3.0 that i'm not aware of.  http://www.w3.org/TR/CSS21/visufx.html#clipping

That said, it might be reasonable to allow clips for fixed-position elements too, in many ways they are similar to absolute positioned elements anyway.
Jun 21, 2013
#16 josa...@chromium.org
Shawn, any eta on possible solution for this blocker?
Jun 21, 2013
#17 shawnsingh@chromium.org
I literally just put a patch up for review and the reviewer is aware of the stable blocker status.  If all the stars align, and the fix lands today/tomorrow, then we should be able to merge it by mon/tues.

We should also request carlanton@ to make sure it's fixed for his situation before we really merge this.

Patch for review is - https://codereview.chromium.org/17155024/
Jun 21, 2013
#18 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=152919

------------------------------------------------------------------------
r152919 | shawnsingh@chromium.org | 2013-06-22T04:24:01.460741Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderLayer.cpp?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/repaint/layer-outline-expected.txt?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/chromium-win/fast/repaint/layer-outline-expected.txt?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/repaint/focus-ring-expected.txt?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/compositing/geometry/clip-expected.txt?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/repaint/layer-outline-horizontal-expected.txt?r1=152919&r2=152918&pathrev=152919
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/chromium-win/fast/repaint/layer-outline-horizontal-expected.txt?r1=152919&r2=152918&pathrev=152919

CSS clip property should not force additional clamping to border box

For a composited layer computing its bounds, there is a code path that
clamps a layer's cliprect to the corresponding element's border box.
However, this is incorrect, since a CSS clip may have valid bounds that
include things outside the border box. So, that clamping should only occur
if there is an overflow clip.

BUG=246728
R=jchaffraix@chromium.org

Review URL: https://codereview.chromium.org/17155024
------------------------------------------------------------------------
Jun 21, 2013
#19 shawnsingh@chromium.org

Patch has landed and seems like it will stick.  Requesting to merge this back to m28 now, but I will not actually merge until it rolls in canary and bakes a bit.
Labels: -Needs-Bisect Merge-Requested
Jun 21, 2013
#20 lafo...@google.com
(No comment was entered for this change.)
Labels: -Merge-Requested Merge-Approved
Jun 24, 2013
#21 shawnsingh@chromium.org
Update - bad news and half-good news.

Bad news: The original gmail bug is not fixed by my first patch.  I believe this patch is still correct, and it fixes a valid bug shown by the reduced test cases, but that doesn't help gmail.  So now I've looked more closely at the original bug, and this is what I'm seeing:

- On mac high-DPI - this is reproducible only with fixed-pos compositing on.
- On chrome OS - this is reproducible with OR without fixed-position compositing, because something else is still forcing the fixed-position send-bar to be composited by overlap.

The half-good news is that I can propose a more isolated workaround than the previous one:  force the formatting bar to be composited using "-webkit-transform: translatez(0)"   This seems to reliably avoid the bug on all platforms.

I will try my best to debug this issue ASAP, on the slim low chance that something can be merged before tomorrow afternoon.  But this is unlikely to happen.

I can turn off fixed-position compositing so that this bug does not arise on gmail on high-DPI mac, but the issue will still exist on chrome OS, so it's not clear to me whether it's really better to turn it off since it gives substantial performance gains to many other situations on these high-DPI platforms.
Cc: wiltz...@chromium.org
Jun 24, 2013
#22 shawnsingh@chromium.org
Hi laforge@ --

I am trying my best to land https://codereview.chromium.org/17653002/ ASAP, which fixes the original gmail bug.

With your permission, we would need to merge both the previous patch (webkit r152919) which you already approved and this new patch.

Are you willing to allow this, even though there will not be much time for it to bake on canary?   This bugfix is needed to avoid the formatting bar from disappearing on gmail for chrome OS and mac retina.

If we cannot merge this, and if gmail cannot workaround this, then we may need to turn off fixed-position compositing for m28 - which would be a separate low-risk patch that needs to be merged to 28.  So, there is a good chance that something needs to be merged either way - either this patch or a patch that turns off fixed-position compositing, so it is at least fixed for mac retina.
Labels: -Merge-Approved Merge-Requested
Jun 24, 2013
#23 shawnsingh@chromium.org
 Issue 253595  has been merged into this issue.
Cc: shawnsingh@chromium.org
Jun 24, 2013
#24 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=152980

------------------------------------------------------------------------
r152980 | shawnsingh@chromium.org | 2013-06-25T03:44:56.849099Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderLayer.cpp?r1=152980&r2=152979&pathrev=152980
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/compositing/geometry/clip-with-shadow-expected.txt?r1=152980&r2=152979&pathrev=152980
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/compositing/geometry/clip-with-shadow.html?r1=152980&r2=152979&pathrev=152980

CSS clip should not be considered when clamping for overflow clip.

Previous patch in webkit rev 152919 partially fixed this, but skipped one code
path that still needed fixing. These code paths are intended to clamp the
backgroundRect to the borderBox, or a slightly inflated borderBox.  It is
incorrect to trigger those code paths when there is only CSS clip and no
overflow clip.  Fixing this bug also results in a tiny refactor where the CSS
clip if-statement is separated from the overflow clip chunks of code.

BUG=246728
R=esprehn@chromium.org

Review URL: https://codereview.chromium.org/17653002
------------------------------------------------------------------------
Jun 25, 2013
#25 lafo...@google.com
We are on the final Beta RC.  Too late in the cycle for this CL (particularly to consider a patch that just landed yesterday).
Labels: -M-28 -Merge-Requested -ReleaseBlock-Stable M-29 Merge-Rejected
Jun 25, 2013
#26 wiltz...@chromium.org
Hmm, OK. In that case we need to either disable the composited fixed positioning flag in M28, have Gmail work around the bug, or sometimes-break the Gmail message editing toolbar on hi DPI displays (retina mac and pixel). 

Shawn is in contact with carlanton about a proposed workaround, but Anthony just a heads up that this might not be done with.
Jun 25, 2013
#27 shawnsingh@chromium.org
I'm told that gmail did get a workaround that should make it to production by end of the week.

So we do not have to worry about disabling fixed-position compositing for m28.

Depending on when m29 branched, there is a good chance I will have to request merging blink r152980 back to m29.   That's the final action needed before we can mark this as fixed.  Thanks for everyone's help and patience =)
Jun 25, 2013
#28 wiltz...@chromium.org
Ah, that's great. Thanks Shawn!
Jun 26, 2013
#29 shawnsingh@chromium.org
Hi Kerz, requesting to merge blink rev 152980 to m29.  

This fixes an issue that has arisen now that we are compositing fixed-position layers.  Among others it affects Gmail web page.  They have kindly worked around the bug for m28, but it would be great to have this fixed for 29.
Labels: -Merge-Rejected Merge-Requested
Jun 26, 2013
#30 kerz@google.com
(No comment was entered for this change.)
Labels: -Merge-Requested Merge-Approved
Jun 26, 2013
#31 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=153084

------------------------------------------------------------------------
r153084 | chrome-bot@google.com | 2013-06-26T18:15:07.288841Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/branches/chromium/1547/LayoutTests/compositing/geometry/clip-with-shadow.html?r1=153084&r2=153083&pathrev=153084
   M http://src.chromium.org/viewvc/blink/branches/chromium/1547/Source/core/rendering/RenderLayer.cpp?r1=153084&r2=153083&pathrev=153084
   A http://src.chromium.org/viewvc/blink/branches/chromium/1547/LayoutTests/compositing/geometry/clip-with-shadow-expected.txt?r1=153084&r2=153083&pathrev=153084

Merge 152980 "CSS clip should not be considered when clamping fo..."

> CSS clip should not be considered when clamping for overflow clip.
> 
> Previous patch in webkit rev 152919 partially fixed this, but skipped one code
> path that still needed fixing. These code paths are intended to clamp the
> backgroundRect to the borderBox, or a slightly inflated borderBox.  It is
> incorrect to trigger those code paths when there is only CSS clip and no
> overflow clip.  Fixing this bug also results in a tiny refactor where the CSS
> clip if-statement is separated from the overflow clip chunks of code.
> 
> BUG=246728
> R=esprehn@chromium.org
> 
> Review URL: https://codereview.chromium.org/17653002

TBR=shawnsingh@chromium.org

Review URL: https://codereview.chromium.org/17894005
------------------------------------------------------------------------
Labels: -Merge-Approved merge-merged-chromium
Jun 26, 2013
#32 shawnsingh@chromium.org
Fix is in version 29.
Status: Fixed
Jun 27, 2013
#33 patri...@chromium.org
(No comment was entered for this change.)
Labels: VerifyIn-29
Jul 17, 2013
#34 bhaves...@chromium.org
Verified.
Google Chrome	29.0.1547.23 (Official Build 211737) dev
Platform	4319.31.0 (Official Build) dev-channel
Status: Verified
Nov 25, 2013
#35 patri...@chromium.org
(No comment was entered for this change.)
Labels: Cr-Blink-Compositing
Sign in to add a comment

Powered by Google Project Hosting