|Summary:||Implement CSSPropertyWebkitColumns in CSSStyleApplyProperty.|
|Product:||WebKit||Reporter:||Luke Macpherson <macpherson>|
|Component:||CSS||Assignee:||Luke Macpherson <macpherson>|
|Severity:||Normal||CC:||darin, levin, macpherson, mitz, rhodovan.u-szeged, webkit.review.bot|
|Version:||528+ (Nightly build)|
|Bug Depends on:||67188|
Description Luke Macpherson 2011-08-28 23:25:03 PDT
Implement CSSPropertyWebkitColumns in CSSStyleApplyProperty.
Comment 2 Darin Adler 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.
Comment 3 Darin Adler 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.
Comment 5 Luke Macpherson 2011-08-29 17:30:27 PDT
Added tests for inherit and initial cases.
Comment 6 Darin Adler 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.
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2011-08-29 18:48:21 PDT
All reviewed patches have been landed. Closing bug.
Comment 9 David Levin 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.
Comment 10 Luke Macpherson 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.
Comment 11 Renata Hodovan 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
Comment 12 Darin Adler 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.
Comment 14 Eric Seidel (no email) 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).