84778 – webkit fails IETC column-width-negative-001.htm

RESOLVED FIXED 84778

webkit fails IETC column-width-negative-001.htm

https://bugs.webkit.org/show_bug.cgi?id=84778

Summary webkit fails IETC column-width-negative-001.htm

Dave Tharp

Reported 2012-04-24 14:25:22 PDT

This one is marked "pass" in the IETC summary grid, but it is actually a failure. If you look closely, you will see little splotches of red between the 4th and 5th "A" and again between the 2nd and 3rd "B". Also the color of all the letters is a brighter green, indicating that the overlaid red letters are missing.

Attachments
Patch (7.07 KB, patch) 2013-01-02 17:51 PST, Uday Kiran	no flags	Details Formatted Diff Diff
Updated Patch (10.56 KB, patch) 2013-01-03 11:57 PST, Uday Kiran	no flags	Details Formatted Diff Diff
Updated Patch (8.94 KB, patch) 2013-01-03 14:08 PST, Uday Kiran	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Uday Kiran

Comment 1 2013-01-02 17:51:57 PST

Created attachment 181124 [details] Patch

Tony Chang

Comment 2 2013-01-03 09:41:22 PST

Comment on attachment 181124 [details] Patch Looking at the spec, it looks like 0 is also invalid: "Specified values must be greater than 0." http://dev.w3.org/csswg/css3-multicol/#column-width Can we make the code match this and add a test for 0? Also, how do Firefox and IE handle 0?

Uday Kiran

Comment 3 2013-01-03 11:57:31 PST

Created attachment 181199 [details] Updated Patch Added test for zero column-width. Other browsers treat zero same like negative value. Thanks for review.

Tony Chang

Comment 4 2013-01-03 12:11:06 PST

Comment on attachment 181199 [details] Updated Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181199&action=review The change looks fine, but the test could be better. I see some red at the edges of the letters due to text antialiasing which is confusing. I would convert this from a ref test to a dumpAsText test (they run faster and are easier to know if it passes or fails) and use fast/js/resources/js-test-pre.js. The change has to do with CSS parsing, so you could simply have a div with column-width: 0 and read the value back out using getComputedStyle (it should be auto). > LayoutTests/fast/multicol/column-width-zero.html:15 > + -moz-column-width: 0em; 0 shouldn't have a unit.

Uday Kiran

Comment 5 2013-01-03 14:08:50 PST

Created attachment 181217 [details] Updated Patch

Tony Chang

Comment 6 2013-01-03 14:10:36 PST

Thanks for fixing this!

WebKit Review Bot

Comment 7 2013-01-03 14:45:29 PST

Comment on attachment 181217 [details] Updated Patch Clearing flags on attachment: 181217 Committed r138746: <http://trac.webkit.org/changeset/138746>

WebKit Review Bot

Comment 8 2013-01-03 14:45:34 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.