[Webkit-unassigned] [Bug 65627] [CSSRegions]RenderFlowThread should display its content using RenderRegion

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 3 12:21:47 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=65627


Dave Hyatt <hyatt at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102797|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #2 from Dave Hyatt <hyatt at apple.com>  2011-08-03 12:21:47 PST ---
(From update of attachment 102797)
View in context: https://bugs.webkit.org/attachment.cgi?id=102797&action=review

> Source/WebCore/rendering/RenderFlowThread.cpp:188
> +void RenderFlowThread::computeLogicalWidth()

This seems odd to me. Does this mean that if you have one narrow region and one wide region, the content will be flowed to the width of the widest region? Shouldn't the width actually vary from region to region?

> Source/WebCore/rendering/RenderFlowThread.cpp:197
> +        int regionWidth = region->contentWidth();
> +        if (regionWidth > width)
> +            width = regionWidth;

I'd just write this as width = max(width, region->contentWidth());

> Source/WebCore/rendering/RenderFlowThread.cpp:211
> +        int regionHeight = region->contentHeight();
> +        height += regionHeight;

Don't really understand the need for a local here. Just add region->contentHeight() to height directly.

> Source/WebCore/rendering/RenderFlowThread.cpp:217
> +void RenderFlowThread::paintRegion(PaintInfo& paintInfo, const LayoutRect& regionRect, const LayoutPoint& paintOffset)

It seems like you're propagating a mistake of columns here by clipping to the region bounds. I don't think overflow should be clipped by default, should it? That's actually a mistake columns make.

Also, what about RenderLayers?  I don't see how you can just do a RenderBlock paint here, since that won't include descendants with layers.

> Source/WebCore/rendering/RenderRegion.cpp:90
> +    // Delegate painting of content in region to RenderFlowThread.
> +    adjustedPaintOffset.move(paddingLeft() + borderLeft(), paddingTop() + borderTop());
> +    m_flowThread->paintRegion(paintInfo, regionRect(), adjustedPaintOffset);

Seems like you just want to paint the flow thread's RenderLayer (it will always have one since you absolutely positioned it) with an offset and clip applied.

I'll give you a pass on clipping for now, since columns make the same mistake, and I guess regions can make the same mistake as well.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list