Bug 152558 - float with media query positioned incorrectly after window resize
Summary: float with media query positioned incorrectly after window resize
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-25 02:59 PST by Alex Studnicka
Modified: 2016-01-06 10:34 PST (History)
8 users (show)

See Also:


Attachments
sample file reproducing the bug (780 bytes, text/html)
2015-12-25 02:59 PST, Alex Studnicka
no flags Details
step 1 screenshot (5.51 KB, image/png)
2015-12-25 03:02 PST, Alex Studnicka
no flags Details
step 2 screenshot (5.84 KB, image/png)
2015-12-25 03:03 PST, Alex Studnicka
no flags Details
step 3 screenshot (5.39 KB, image/png)
2015-12-25 03:03 PST, Alex Studnicka
no flags Details
Test reduction (209 bytes, text/html)
2015-12-26 13:20 PST, zalan
no flags Details
Patch (4.90 KB, patch)
2015-12-27 10:10 PST, zalan
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
expected:
--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"

actual:
--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]
Patch
Comment 9 Simon Fraser (smfr) 2015-12-27 11:07:06 PST
Comment on attachment 267945 [details]
Patch

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>