Bug 117693 – Add support for the column-fill property

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Morten Stenshorne]

WebKit doesn't support the column-fill property. Instead it behaves as if 'column-fill' is set to 'auto'. This makes balancing impossible if height is specified.

[Morten Stenshorne]

Created attachment 204806 [details]
Test case

[Morten Stenshorne]

Created attachment 204807 [details]
Patch

[Morten Stenshorne]

Note that the patch is only for the (new) region based columns implementation.

[Build Bot]

Comment on attachment 204807 [details]
Patch

Attachment 204807 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/876137

New failing tests:
fast/repaint/table-cell-collapsed-border-scroll.html
fast/multicol/newmulticol/fixed-height-fill-balance.html

[Build Bot]

Created attachment 204825 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Morten Stenshorne]

Created attachment 204841 [details]
Patch

[Morten Stenshorne]

New patch, attempted to fix fast/multicol/newmulticol/fixed-height-fill-balance.html on Mac (rounding error?)

No idea why fast/repaint/table-cell-collapsed-border-scroll.html failed.

[Build Bot]

Comment on attachment 204841 [details]
Patch

Attachment 204841 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/895233

New failing tests:
fast/repaint/table-cell-collapsed-border-scroll.html
fast/multicol/newmulticol/fixed-height-fill-balance.html

[Build Bot]

Created attachment 204870 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 204841 [details]
Patch

Attachment 204841 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/858531

New failing tests:
fast/repaint/table-cell-collapsed-border-scroll.html
fast/multicol/newmulticol/fixed-height-fill-balance.html

[Build Bot]

Created attachment 204874 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 204841 [details]
Patch

Attachment 204841 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/865322

New failing tests:
fast/multicol/newmulticol/fixed-height-fill-balance.html

[Build Bot]

Created attachment 204890 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Morten Stenshorne]

Created attachment 204980 [details]
Patch

[Morten Stenshorne]

Another attempt to make the test pass.

[Dave Hyatt]

Comment on attachment 204980 [details]
Patch

Just to make sure, the behavior of the WebKit pagination API (which is built using columns) needs to be that you fill by default rather than balance. Does multicolumn applied to the RenderView properly set fill? Look at the method in RenderStyle that sets column styles based off the pagination mode, since I think in that case you want to also always set fill to auto to avoid balancing.

[Morten Stenshorne]

This patch only adds support for column-fill for the region based multicol implementation, and RenderMultiColumnBlock currently only supports real multicol. It doesn't support paged overflow. Even if it did (it should, at some point), it wouldn't be a problem, because paged overflow means that you get a used column-count of 1, so no balancing could possibly take place.

BTW, RenderView doesn't inherit from RenderMultiColumnBlock. That would have been weird, anyway. Not sure what to do to get proper region based multicol support on a paginated RenderView (I assume this is desirable at some point). Probably need to get rid of RenderMultiColumnBlock and merge it into RenderBlock.

In any case, would overriding computed style for column-fill be very healthy? Wouldn't it be better to let the renderer figure out how to treat it?

[Dave Hyatt]

Comment on attachment 204980 [details]
Patch

r=me

[Morten Stenshorne]

Created attachment 212690 [details]
Patch

[Morten Stenshorne]

The previous patch failed to land for some reason (looks like a bot problem). I've rebased master and uploaded a new patch. Please take a look.

[Morten Stenshorne]

@dhyatt: please see previous comment.

[Morten Stenshorne]

Created attachment 214244 [details]
Patch

[Morten Stenshorne]

Can someone review+ and cq+ this one for me (I just rebased master again)?

[Morten Stenshorne]

Created attachment 214282 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 214282 [details]
Patch

Clearing flags on attachment: 214282

Committed r157458: <http://trac.webkit.org/changeset/157458>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.