Bug 118531 - [CSS Regions] In a region chain with auto-height regions, lines get their length based only on the first region
Summary: [CSS Regions] In a region chain with auto-height regions, lines get their len...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andrei Bucur
URL:
Keywords:
Depends on: 118595
Blocks: 57312
  Show dependency treegraph
 
Reported: 2013-07-10 08:39 PDT by Mihai Balan
Modified: 2013-07-12 06:35 PDT (History)
5 users (show)

See Also:


Attachments
Test-case (827 bytes, text/html)
2013-07-10 08:39 PDT, Mihai Balan
no flags Details
Patch (13.73 KB, patch)
2013-07-11 07:51 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (13.59 KB, patch)
2013-07-11 08:22 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Balan 2013-07-10 08:39:34 PDT
Created attachment 206392 [details]
Test-case

If a region chain has regions with height: auto and specified widths, line breaking will not take into consideration the width of the current region, but the width of the first region. This can lead to text overflowing a region (if narrower than the first region) or to regions unnecessarily tall and with unneeded whitespace (if wider than the first region).
Comment 1 Andrei Bucur 2013-07-11 07:51:11 PDT
Created attachment 206463 [details]
Patch
Comment 2 Alexandru Chiculita 2013-07-11 08:02:38 PDT
Comment on attachment 206463 [details]
Patch

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.
Comment 3 Andrei Bucur 2013-07-11 08:22:23 PDT
Created attachment 206467 [details]
Patch
Comment 4 WebKit Commit Bot 2013-07-11 09:18:34 PDT
Comment on attachment 206467 [details]
Patch

Clearing flags on attachment: 206467

Committed r152572: <http://trac.webkit.org/changeset/152572>
Comment 5 WebKit Commit Bot 2013-07-11 09:18:37 PDT
All reviewed patches have been landed.  Closing bug.