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

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Mihnea Ovidenie <mihnea at adobe.com>  2011-08-04 08:50:48 PST ---
(In reply to comment #2)
> (From update of attachment 102797 [details])
> 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?

The width will indeed vary from region to region, we thought we will fix that in the following patches. Should we add a test for this issue with this patch?

> 
> > 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());

Sure, i missed that.

> 
> > 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.

Right.

> 
> > 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.

I thought that adding support for RenderLayers should be another patch, what do you think? 

> 
> > 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.

Should i add a bug for this issue? Maybe fill a bug for columns too?

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