Bug 117693 - Add support for the column-fill property
Summary: Add support for the column-fill property
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Morten Stenshorne
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-17 03:38 PDT by Morten Stenshorne
Modified: 2013-10-15 11:43 PDT (History)
12 users (show)

See Also:


Attachments
Test case (477 bytes, text/html)
2013-06-17 03:43 PDT, Morten Stenshorne
no flags Details
Patch (135.12 KB, patch)
2013-06-17 04:12 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (626.00 KB, application/zip)
2013-06-17 08:43 PDT, Build Bot
no flags Details
Patch (135.15 KB, patch)
2013-06-17 11:10 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (734.50 KB, application/zip)
2013-06-17 17:42 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (517.66 KB, application/zip)
2013-06-17 18:40 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (652.27 KB, application/zip)
2013-06-18 01:57 PDT, Build Bot
no flags Details
Patch (135.30 KB, patch)
2013-06-19 02:58 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Patch (132.84 KB, patch)
2013-09-26 03:52 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Patch (132.91 KB, patch)
2013-10-15 02:12 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Patch (132.90 KB, patch)
2013-10-15 11:14 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Morten Stenshorne 2013-06-17 03:38:43 PDT
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.
Comment 1 Morten Stenshorne 2013-06-17 03:43:27 PDT
Created attachment 204806 [details]
Test case
Comment 2 Morten Stenshorne 2013-06-17 04:12:16 PDT
Created attachment 204807 [details]
Patch
Comment 3 Morten Stenshorne 2013-06-17 04:12:48 PDT
Note that the patch is only for the (new) region based columns implementation.
Comment 4 Build Bot 2013-06-17 08:43:19 PDT
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
Comment 5 Build Bot 2013-06-17 08:43:20 PDT
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
Comment 6 Morten Stenshorne 2013-06-17 11:10:52 PDT
Created attachment 204841 [details]
Patch
Comment 7 Morten Stenshorne 2013-06-17 11:12:10 PDT
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.
Comment 8 Build Bot 2013-06-17 17:42:25 PDT
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
Comment 9 Build Bot 2013-06-17 17:42:27 PDT
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
Comment 10 Build Bot 2013-06-17 18:40:52 PDT
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
Comment 11 Build Bot 2013-06-17 18:40:54 PDT
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
Comment 12 Build Bot 2013-06-18 01:57:32 PDT
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
Comment 13 Build Bot 2013-06-18 01:57:35 PDT
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
Comment 14 Morten Stenshorne 2013-06-19 02:58:40 PDT
Created attachment 204980 [details]
Patch
Comment 15 Morten Stenshorne 2013-06-19 02:59:04 PDT
Another attempt to make the test pass.
Comment 16 Dave Hyatt 2013-08-05 13:49:50 PDT
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.
Comment 17 Morten Stenshorne 2013-08-06 04:08:49 PDT
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?
Comment 18 Dave Hyatt 2013-08-27 11:25:45 PDT
Comment on attachment 204980 [details]
Patch

r=me
Comment 19 Morten Stenshorne 2013-09-26 03:52:39 PDT
Created attachment 212690 [details]
Patch
Comment 20 Morten Stenshorne 2013-09-26 03:57:25 PDT
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.
Comment 21 Morten Stenshorne 2013-09-26 03:59:02 PDT
@dhyatt: please see previous comment.
Comment 22 Morten Stenshorne 2013-10-15 02:12:27 PDT
Created attachment 214244 [details]
Patch
Comment 23 Morten Stenshorne 2013-10-15 02:13:56 PDT
Can someone review+ and cq+ this one for me (I just rebased master again)?
Comment 24 Morten Stenshorne 2013-10-15 11:14:04 PDT
Created attachment 214282 [details]
Patch
Comment 25 WebKit Commit Bot 2013-10-15 11:43:54 PDT
Comment on attachment 214282 [details]
Patch

Clearing flags on attachment: 214282

Committed r157458: <http://trac.webkit.org/changeset/157458>
Comment 26 WebKit Commit Bot 2013-10-15 11:43:58 PDT
All reviewed patches have been landed.  Closing bug.