Bug 152558

Summary: float with media query positioned incorrectly after window resize
Product: WebKit Reporter: Alex Studnicka <astudnicka>
Component: CSSAssignee: zalan <zalan>
Severity: Normal CC: astudnicka, commit-queue, esprehn+autocc, glenn, hyatt, kondapallykalyan, simon.fraser, zalan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Description Flags
sample file reproducing the bug
step 1 screenshot
step 2 screenshot
step 3 screenshot
Test reduction
Patch simon.fraser: review+

Description Alex Studnicka 2015-12-25 02:59:49 PST
Created attachment 267916 [details]
sample file reproducing the bug

Steps to reproduce:
1. Open attached html file in a window with width larger than 550px
2. Resize the window to less than 550px
3. Resize the window back to more than 550px

Expected result:
Page should look exactly like before resizing

Actual result:
div with float: right is positioned incorrectly
Comment 1 Alex Studnicka 2015-12-25 03:02:44 PST
Created attachment 267917 [details]
step 1 screenshot
Comment 2 Alex Studnicka 2015-12-25 03:03:01 PST
Created attachment 267918 [details]
step 2 screenshot
Comment 3 Alex Studnicka 2015-12-25 03:03:21 PST
Created attachment 267919 [details]
step 3 screenshot
Comment 4 zalan 2015-12-25 10:01:38 PST
Both FF and Chrome position the float correctly.
Comment 5 zalan 2015-12-26 13:20:11 PST
Created attachment 267928 [details]
Test reduction
Comment 6 zalan 2015-12-26 13:26:52 PST
--AG--- --        RenderBlock  (0.00, 0.00) (1004.00, 18.00) renderer->(0x11e5e9228)
------- --          SPAN RenderInline renderer->(0x11d1dfb60) node->(0x11d1df958)
------- --            #text RenderText renderer->(0x11d1f5540) node->(0x11d1d32d0) length->(5) "above"
----F-- --          DIV RenderBlock  (973.78, 0.00) (30.22, 18.00) renderer->(0x11e5e9730) node->(0x11d1df9c0)
------- --            #text RenderText renderer->(0x11e7fa000) node->(0x11d1d3320) length->(5) "right"
------- --        DIV RenderBlock  (0.00, 18.00) (1004.00, 18.00) renderer->(0x11e5e97e8) node->(0x11d1dfa28)
------- --          #text RenderText renderer->(0x11e7fa1e0) node->(0x11d1d3370) length->(5) "below"

--AG--- --        RenderBlock  (0.00, 0.00) (1004.00, 18.00) renderer->(0x1187eecf0)
------- --          SPAN RenderInline renderer->(0x1187f1f08) node->(0x1187f1a90)
------- --            #text RenderText renderer->(0x1187a3840) node->(0x1187f2b40) length->(5) "above"
----F-- --        DIV RenderBlock  (973.78, 18.00) (30.22, 18.00) renderer->(0x1187ee2e0) node->(0x1187f1af8)
------- --          #text RenderText renderer->(0x1187a3900) node->(0x1187f2b90) length->(5) "right"
------- --        DIV RenderBlock  (0.00, 18.00) (1004.00, 18.00) renderer->(0x1187eeb80) node->(0x1187f1b60)
------- --          #text RenderText renderer->(0x1187a3a20) node->(0x1187f2be0) length->(5) "below"

Seems like the anonymous RenderBlock (for the span) triggers this behavior.
Comment 7 zalan 2015-12-26 21:59:40 PST
RenderBlock::addChildIgnoringContinuation says:
"// If we're inserting an inline (or floated) child but all of our children are blocks, then we have to make sure
// it is put into an anomyous block box. We try to use an existing anonymous box if possible, otherwise
// a new one is created and inserted into our list of children in the appropriate position."
However when we are transforming an existing block renderer to floated, we don't follow this rule and the floated renderer ends up with the wrong parent.
The fix is to apply the same logic when this style change happens.
Something like this (but probably with a bit more limited scope)
+    // This renderer might need a new parent.
+    if (m_style->isFloating() && previousSibling() && previousSibling()->isAnonymousBlock())
+        downcast<RenderBoxModelObject>(*parent()).moveChildTo(&downcast<RenderBoxModelObject>(*previousSibling()), this);
right after calling RenderElement::removeAnonymousWrappersForInlinesIfNecessary() in RenderElement::styleDidChange() when s_noLongerAffectsParentBlock is true. 
This puts the floated renderer on the correct line (other "out of flow" style changes should probably trigger the same logic, though they don't impact rendering the same way float does).
Comment 8 zalan 2015-12-27 10:10:44 PST
Created attachment 267945 [details]
Comment 9 Simon Fraser (smfr) 2015-12-27 11:07:06 PST
Comment on attachment 267945 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=267945&action=review

> Source/WebCore/rendering/RenderElement.cpp:1002
> +        // It copies the logic of RenderBlock::addChildIgnoringContinuation

Can the code be shared?
Comment 10 zalan 2016-01-06 10:34:21 PST
Committed r194645: <http://trac.webkit.org/changeset/194645>