Bug 117693

Summary:

Add support for the column-fill property

Product:

WebKit

Reporter:

Morten Stenshorne <mstensho>

Component:

Layout and Rendering

Assignee:

Morten Stenshorne <mstensho>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, hyatt, kondapallykalyan, macpherson, menard, rniwa, syoichi, WebkitBugTracker

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Test case	none
Patch	none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion	none
Patch	none
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2	none
Patch	none
Patch	none
Patch	none
Patch	none

Morten Stenshorne

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

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
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Morten Stenshorne

Comment 1 2013-06-17 03:43:27 PDT

Created attachment 204806 [details] Test case

Morten Stenshorne

Comment 2 2013-06-17 04:12:16 PDT

Created attachment 204807 [details] Patch

Morten Stenshorne

Comment 3 2013-06-17 04:12:48 PDT

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

Build Bot

Comment 4 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

Build Bot

Comment 5 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

Morten Stenshorne

Comment 6 2013-06-17 11:10:52 PDT

Created attachment 204841 [details] Patch

Morten Stenshorne

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

Build Bot

Comment 8 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

Build Bot

Comment 9 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

Build Bot

Comment 10 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

Build Bot

Comment 11 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

Build Bot

Comment 12 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

Build Bot

Comment 13 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

Morten Stenshorne

Comment 14 2013-06-19 02:58:40 PDT

Created attachment 204980 [details] Patch

Morten Stenshorne

Comment 15 2013-06-19 02:59:04 PDT

Another attempt to make the test pass.

Dave Hyatt

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

Morten Stenshorne

Comment 17 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?

Dave Hyatt

Comment 18 2013-08-27 11:25:45 PDT

Comment on attachment 204980 [details] Patch r=me

Morten Stenshorne

Comment 19 2013-09-26 03:52:39 PDT

Created attachment 212690 [details] Patch

Morten Stenshorne

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

Morten Stenshorne

Comment 21 2013-09-26 03:59:02 PDT

@dhyatt: please see previous comment.