Bug 12158

Summary: Implement CSS3 multi column support
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Implement parsing of the properties
none
Better patch for parsing of the properties.
none
Revised patch addressing Eric and MItz's review comments. eric: review+

Description Dave Hyatt 2007-01-08 00:15:28 PST
This is a bug to track CSS3 multicol support.
Comment 1 Dave Hyatt 2007-01-08 00:16:29 PST
Created attachment 12294 [details]
Implement parsing of the properties

Note that column-width and column-gap are a lot like z-index, and while adding support for them, I discovered a bug with z-index.  That's why this patch contains a z-index fix and test case. :)
Comment 2 Dave Hyatt 2007-01-08 00:20:42 PST
Created attachment 12295 [details]
Better patch for parsing of the properties.

Fixed an error in the columns shorthand parsing and made sure column-count is restricted in the parser to be non-negative.
Comment 3 mitz 2007-01-08 00:24:06 PST
Comment on attachment 12295 [details]
Better patch for parsing of the properties.

+        const int properties[2] = { CSS_PROP__WEBKIT_COLUMN_WIDTH, CSS_PROP__WEBKIT_COLUMN_COUNT };
+        return parseShorthand(propId, properties, 3, important);

Should be 2 instead of 3.
Comment 4 mitz 2007-01-08 00:37:03 PST
+    static Length initialColumnWidth() { return Length(); }

Is this supposed to be a Length? Seems to be a float everywhere else.
Comment 5 mitz 2007-01-08 00:39:24 PST
Actually, I guess the question is, is it really supposed to be a float everywhere else.
Comment 6 Dave Hyatt 2007-01-08 00:46:05 PST
Created attachment 12296 [details]
Revised patch addressing Eric and MItz's review comments.
Comment 7 Eric Seidel (no email) 2007-01-08 00:51:53 PST
Comment on attachment 12296 [details]
Revised patch addressing Eric and MItz's review comments.

My thoughts:

1.  You'll need lots more tests... eventually.
2.  ideally the z-index thing would be separate, but it's OK as is.
3.  Some of this new code (including the z-index stuff) could really be a macro to avoid more copy/paste trouble.  But it's OK as is.

Looks good.
Comment 8 Sam Weinig 2007-01-08 06:14:28 PST
Landed in r18660 (multi column) and r18661 (z-index).