Bug 84778 - webkit fails IETC column-width-negative-001.htm
Summary: webkit fails IETC column-width-negative-001.htm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://samples.msdn.microsoft.com/iet...
Keywords:
Depends on:
Blocks: 76198
  Show dependency treegraph
 
Reported: 2012-04-24 14:25 PDT by Dave Tharp
Modified: 2013-01-03 14:45 PST (History)
17 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Tharp 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.
Comment 1 Uday Kiran 2013-01-02 17:51:57 PST
Created attachment 181124 [details]
Patch
Comment 2 Tony Chang 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?
Comment 3 Uday Kiran 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.
Comment 4 Tony Chang 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.
Comment 5 Uday Kiran 2013-01-03 14:08:50 PST
Created attachment 181217 [details]
Updated Patch
Comment 6 Tony Chang 2013-01-03 14:10:36 PST
Thanks for fixing this!
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2013-01-03 14:45:34 PST
All reviewed patches have been landed.  Closing bug.