Bug 50970

Summary: REGRESSION: Floated text is not rendered after r73385
Product: WebKit Reporter: Mihai Parparita <mihaip>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: hyatt, jamesr, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 49220    
Attachments:
Description Flags
Test Case
none
Screenshot of testcase in IE9 in IE9 mode
none
Patch mitz: review+

Mihai Parparita
Reported 2010-12-13 14:07:12 PST
Test case: http://persistent.info/webkit/test-cases/crbug-66564.html. Verified locally that building before http://trac.webkit.org/changeset/73385 results in the text being displayed, but not when building after. Initially reported on the Chromium side as http://crbug.com/66564
Attachments
Test Case (333 bytes, text/html)
2010-12-14 13:10 PST, Dave Hyatt 		no flags
Details
Screenshot of testcase in IE9 in IE9 mode (19.35 KB, image/png)
2010-12-14 13:32 PST, Tony Gentilcore 		no flags
Details
Patch (45.40 KB, patch)
2010-12-14 14:39 PST, Dave Hyatt 		mitz: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
James Robinson
Comment 1 2010-12-13 14:12:35 PST
I suspect this is related to bug 50963
James Robinson
Comment 2 2010-12-13 15:28:23 PST
(In reply to comment #1) > I suspect this is related to bug 50963 Nope, it's something else.
James Robinson
Comment 3 2010-12-13 16:33:33 PST
We're inserting the float into the float list for the <body>, but the enclosing self render layer for the float is the layer for the relpositioned inline <div>. The RenderLayer for the body doesn't paint the float (the enclosing self painting layers are different) and the RenderLayer for the relpos div doesn't paint the float because it's not in the floating object list for anything within the layer.
James Robinson
Comment 4 2010-12-13 17:41:00 PST
This fixes the issue: --- a/WebCore/rendering/RenderBlock.cpp +++ b/WebCore/rendering/RenderBlock.cpp @@ -3580,8 +3580,7 @@ int RenderBlock::addOverhangingFloats(RenderBlock* child, int logicalLeftOffset, } m_floatingObjects->append(floatingObj); } - } else { - if (makeChildPaintOtherFloats && !r->m_shouldPaint && !r->m_renderer->hasSelfPaintingLayer() && + } else if (makeChildPaintOtherFloats && !r->m_shouldPaint && !r->m_renderer->hasSelfPaintingLayer() && r->m_renderer->isDescendantOf(child) && r->m_renderer->enclosingLayer() == child->enclosingLayer()) { // The float is not overhanging from this block, so if it is a descendant of the child, the child should // paint it (the other case is that it is intruding into the child), unless it has its own layer or enclosing @@ -3590,12 +3589,11 @@ int RenderBlock::addOverhangingFloats(RenderBlock* child, int logicalLeftOffset, // it should paint. r->m_shouldPaint = true; } - - // Since the float doesn't overhang, it didn't get put into our list. We need to go ahead and add its overflow in to the - // child now. - if (r->m_isDescendant) - child->addOverflowFromChild(r->m_renderer, IntSize(r->left() + r->m_renderer->marginLeft(), r->top() + r->m_renderer->marginTop())); } + // Since the float doesn't overhang, it didn't get put into our list. We need to go ahead and add its overflow in to the + // child now. + if (r->m_isDescendant) + child->addOverflowFromChild(r->m_renderer, IntSize(r->left() + r->m_renderer->marginLeft(), r->top() + r->m_renderer->marginTop())); } return lowestFloatLogicalBottom; It's essentially reverting all the changes that 73385 made to addOverhangingFloats() except for the floatingObj->m_isDescendant = true line. I think this is a case for Hyatt to decide what to do :)
Dave Hyatt
Comment 5 2010-12-14 13:09:17 PST
This is interesting. Really the issue is just who exactly is supposed to paint that float? Should the relatively positioned inline paint the float, or should the block paint the float? The fact that the block paints it means the float is effectively ignoring the fact that it is inside the relative positioned span. See the attached test case.
Dave Hyatt
Comment 6 2010-12-14 13:10:35 PST
Created attachment 76561 [details] Test Case In the attached test case, Firefox and WebKit disagree. Firefox is inconsistent though, since it is applying the stacking order based off the float being inside the relative positioned div, but it's not applying the relative positioned offset to the div. Opera agrees with WebKit. Maybe someone could try IE9.
Tony Gentilcore
Comment 7 2010-12-14 13:32:57 PST
Created attachment 76564 [details] Screenshot of testcase in IE9 in IE9 mode
Dave Hyatt
Comment 8 2010-12-14 14:39:58 PST
Created attachment 76572 [details] Patch For now just retain our behavior of having the blocks always paint floats. I actually think IE9's rendering makes more sense than what we do, but changing to match that rendering is more involved.
Dave Hyatt
Comment 9 2010-12-14 15:02:05 PST
Fixed in r74063.
Note You need to log in before you can comment on or make changes to this bug.