Bug 67106

Summary: Implement CSSPropertyWebkitColumns in CSSStyleApplyProperty.
Product: WebKit Reporter: Luke Macpherson <macpherson>
Component: CSSAssignee: Luke Macpherson <macpherson>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, levin, macpherson, mitz, rhodovan.u-szeged, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 67188    
Bug Blocks: 67685    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Luke Macpherson
Reported 2011-08-28 23:25:03 PDT
Implement CSSPropertyWebkitColumns in CSSStyleApplyProperty.
Attachments
Patch (3.81 KB, patch)
2011-08-28 23:28 PDT, Luke Macpherson 		no flags
DetailsFormatted DiffDiff
Patch (24.68 KB, patch)
2011-08-29 17:29 PDT, Luke Macpherson 		no flags
DetailsFormatted DiffDiff
Patch (27.47 KB, patch)
2011-09-04 21:53 PDT, Luke Macpherson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Luke Macpherson
Comment 1 2011-08-28 23:28:38 PDT
Created attachment 105462 [details] Patch
Darin Adler
Comment 2 2011-08-29 11:00:10 PDT
Comment on attachment 105462 [details] Patch Are there tests covering inheriting and initial for these properties? Since the new way of doing this is so different from the old way, I was wondering how we were checking that behavior is still the same? If there aren’t tests, it seems it would be really straightforward to make them as we go.
Darin Adler
Comment 3 2011-08-29 11:00:34 PDT
The thing to keep in mind is that the bar is higher for test coverage when we are doing a refactoring.
Luke Macpherson
Comment 4 2011-08-29 17:29:43 PDT
Created attachment 105552 [details] Patch
Luke Macpherson
Comment 5 2011-08-29 17:30:27 PDT
Added tests for inherit and initial cases.
Darin Adler
Comment 6 2011-08-29 17:45:16 PDT
Comment on attachment 105552 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105552&action=review > LayoutTests/fast/multicol/initial-column-values.html:8 > +<div style="-webkit-columns: initial;"> This test would be even better if it used computed style and dumpAsText so it work without platform-specific results.
WebKit Review Bot
Comment 7 2011-08-29 18:48:16 PDT
Comment on attachment 105552 [details] Patch Clearing flags on attachment: 105552 Committed r94037: <http://trac.webkit.org/changeset/94037>
WebKit Review Bot
Comment 8 2011-08-29 18:48:21 PDT
All reviewed patches have been landed. Closing bug.
David Levin
Comment 9 2011-08-29 22:31:44 PDT
(In reply to comment #6) > (From update of attachment 105552 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105552&action=review > > > LayoutTests/fast/multicol/initial-column-values.html:8 > > +<div style="-webkit-columns: initial;"> > > This test would be even better if it used computed style and dumpAsText so it work without platform-specific results. Luke, please consider this strongly. Platform specific results are a huge maintenance burden on the project.
Luke Macpherson
Comment 10 2011-08-29 23:06:41 PDT
Yeah, will do. I was surprised it got the cq+ with that request, which seemed a good idea to me.
Renata Hodovan
Comment 11 2011-08-30 02:30:27 PDT
The new tests are failed on Qt Linux Release bot. The bug is filed in: https://bugs.webkit.org/show_bug.cgi?id=67188
Darin Adler
Comment 12 2011-08-30 07:50:18 PDT
(In reply to comment #10) > Yeah, will do. I was surprised it got the cq+ with that request, which seemed a good idea to me. You are more than welcome to follow up by fixing the test.
Luke Macpherson
Comment 13 2011-09-04 21:53:32 PDT
Created attachment 106304 [details] Patch
Eric Seidel (no email)
Comment 14 2011-09-06 15:45:13 PDT
Comment on attachment 106304 [details] Patch Cleared review? from attachment 106304 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.