Summary: | Implement CSS3 multi column support | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||||||
Component: | CSS | Assignee: | Dave Hyatt <hyatt> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | mitz | ||||||||
Priority: | P2 | ||||||||||
Version: | 420+ | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Attachments: |
|
Description
Dave Hyatt
2007-01-08 00:15:28 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. :)
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 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.
+ static Length initialColumnWidth() { return Length(); } Is this supposed to be a Length? Seems to be a float everywhere else. Actually, I guess the question is, is it really supposed to be a float everywhere else. Created attachment 12296 [details]
Revised patch addressing Eric and MItz's review comments.
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.
|