Summary: | Implement CSSPropertyWebkitColumns in CSSStyleApplyProperty. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Luke Macpherson <macpherson> | ||||||||
Component: | CSS | Assignee: | 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
Luke Macpherson
2011-08-28 23:25:03 PDT
Created attachment 105462 [details]
Patch
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.
The thing to keep in mind is that the bar is higher for test coverage when we are doing a refactoring. Created attachment 105552 [details]
Patch
Added tests for inherit and initial cases. 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 on attachment 105552 [details] Patch Clearing flags on attachment: 105552 Committed r94037: <http://trac.webkit.org/changeset/94037> All reviewed patches have been landed. Closing bug. (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. Yeah, will do. I was surprised it got the cq+ with that request, which seemed a good idea to me. The new tests are failed on Qt Linux Release bot. The bug is filed in: https://bugs.webkit.org/show_bug.cgi?id=67188 (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. Created attachment 106304 [details]
Patch
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). |