Bug 93754 - REGRESSION (r122342): stacking order of floating divs is incorrect (LayoutTests/fast/overflow/overflow-float-stacking.html)
Summary: REGRESSION (r122342): stacking order of floating divs is incorrect (LayoutTes...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Julien Chaffraix
URL:
Keywords: InRadar, LayoutTestFailure, Regression
Depends on:
Blocks: 111734
  Show dependency treegraph
 
Reported: 2012-08-10 16:45 PDT by Kiran Muppala
Modified: 2013-04-08 11:01 PDT (History)
10 users (show)

See Also:


Attachments
Proposed fix: broaded the has-self-painting-layer-descendant flag to cover also overlay scrollbars. (16.28 KB, patch)
2013-02-01 16:52 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Updated patch. (16.67 KB, patch)
2013-02-27 09:50 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kiran Muppala 2012-08-10 16:45:35 PDT
A visual inspection of layout test fast/overflow/overflow-float-stacking.html showed that of the two floating divs, the second one is behind the first one after the change in r122342, where as it is in front of the first one prior to that.  The text in the second div, says that it should be above the first div.  Surprisingly the test is still passing when run through run-webkit-tests.  Hence, filing this bug to track the issue.
Comment 1 Kiran Muppala 2012-08-10 16:47:10 PDT
<rdar://problem/12078874>
Comment 2 Adele Peterson 2012-09-20 17:00:02 PDT
Julian, are you looking into this regression?  Should we consider rolling out r122342?
Comment 3 Julien Chaffraix 2012-09-21 11:25:53 PDT
(In reply to comment #2)
> Julian, are you looking into this regression?

I looked at the regression and I think this is an unintended consequence of making overlay scrollbars' layers self-painting. Due to how we handle self-painting layers, we don't necessarily paint their renderer in tree order anymore: you can actually experience the painting reversal by setting "opacity" or "position" on the green div on any passing build. The reversal matches FF and Opera for "opacity" or "position" but not so much for "overflow".

> Should we consider rolling out r122342?

Rolling it out would break back overlay scrollbars but the new behavior is bad.
Comment 4 Sam Weinig 2013-01-30 16:43:42 PST
(In reply to comment #3)
> (In reply to comment #2)
> > Julian, are you looking into this regression?
> 
> I looked at the regression and I think this is an unintended consequence of making overlay scrollbars' layers self-painting. D
ue to how we handle self-painting layers, we don't necessarily paint their renderer in tree order anymore: you can actually experience the painting reversal by setting "opacity" or "position" on the green div on any passing build. The reversal matches FF and Opera for "opacity" or "position" but not so much for "overflow".
> 
> > Should we consider rolling out r122342?
> 
> Rolling it out would break back overlay scrollbars but the new behavior is bad.

Julien, any progress?
Comment 5 Julien Chaffraix 2013-01-31 16:32:22 PST
> Julien, any progress?

None, this bug fell off my queue. The painting inversion is a bug in self-painting layers that we already experience in some context, I will have to think harder about how to fix this bug without regressing overlay scrollbars.
Comment 6 Julien Chaffraix 2013-02-01 16:52:41 PST
Created attachment 186187 [details]
Proposed fix: broaded the has-self-painting-layer-descendant flag to cover also overlay scrollbars.
Comment 7 Build Bot 2013-02-01 19:46:30 PST
Comment on attachment 186187 [details]
Proposed fix: broaded the has-self-painting-layer-descendant flag to cover also overlay scrollbars.

Attachment 186187 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16330488
Comment 8 Julien Chaffraix 2013-02-27 09:50:16 PST
Created attachment 190547 [details]
Updated patch.
Comment 9 Julien Chaffraix 2013-04-08 11:01:18 PDT
Comment on attachment 190547 [details]
Updated patch.

Simon mentioned on IRC that this patch should be split between the should-be-painted refactoring and the actual fix to ease the review. As part of that, it would be nice to define or outline better which layers are non-self-painting.