[webkit-reviews] review granted: [Bug 118531] [CSS Regions] In a region chain with auto-height regions, lines get their length based only on the first region : [Attachment 206463] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 11 08:00:35 PDT 2013


Alexandru Chiculita <achicu at adobe.com> has granted Andrei Bucur
<abucur at adobe.com>'s request for review:
Bug 118531: [CSS Regions] In a region chain with auto-height regions, lines get
their length based only on the first region
https://bugs.webkit.org/show_bug.cgi?id=118531

Attachment 206463: Patch
https://bugs.webkit.org/attachment.cgi?id=206463&action=review

------- Additional Comments from Alexandru Chiculita <achicu at adobe.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=206463&action=review


Looks good.

> Source/WebCore/rendering/RenderFlowThread.cpp:267
> +    ASSERT(computedValues.m_extent >= 0);

Looks like having no regions will hit this assert computedValues.m_extent >= 0

> Source/WebCore/rendering/RenderFlowThread.h:171
> +    static LayoutUnit maxFlowThreadSize() { return LayoutUnit::max() / 2; }

nit: I would not repeat the flowThread in the name and I think we should
mention that it refers to the logicalHeight of the flow. What about
maxLogicalHeight() ? Add a comment that it is used to estimate the size of the
flow.


More information about the webkit-reviews mailing list