In the following HTML table, the 'table' element has two CSS2 declarations. In CSS2, the 'border-spacing' property cannot have negative lengths. Accordingly, a CSS2 user agent must ignore the declaration. But, Mozilla 0.9.4 accepts it. <table border="1" style="border-collapse: separate; border-spacing: -0.5em"> <tr> <td>ABCDEFG</td> <td>HIJKLMN</td> </tr> <tr> <td>OPQRSTU</td> <td>VWXYZ</td> </tr> </table> References: http://www.w3.org/TR/REC-CSS2/tables.html#propdef-border-spacing http://www.w3.org/TR/html401/struct/tables.html
Indeed. Over to style system....
Assignee: karnaze → dbaron
Component: HTMLTables → Style System
QA Contact: amar → ian
Yep. Confirming. Either we need a new variant for non-negative lengths or ParseBorderSpacing needs to do the checking itself.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Summary: 'border-spacing' property in CSS2 → 'border-spacing' property in CSS2 cannot have negative lengths
We should probably do whatever we do for padding, which also cannot accept negative lengths.
Looks like that would just be a matter of replacing ParseVariant with ParsePositiveVariant in CSSParserImpl::ParseBorderSpacing Attaching patch
Keywords: css2, mozilla0.9.5, patch, review
The way you use ExpectEndProperty will mess up error reporting. Perhaps it would be better to call ExpectEndProperty in the else of the second ParsePositiveVariant check and move the AppendValue calls for the one-value case there as well?
Well, it looks like ParsePositiveVariant() will eat up tokens... so something like: border-spacing: 0.5em -2em; would succeed on the first ParsePositiveVariant, fail on the second. Now what do we do? Just use 0.5em for both dimensions? We have no way of telling whether the second ParsePositiveVariant failed because there was nothing there to parse or because it parsed an illegal value.
Would it make sense for ParsePositiveVariant to UngetToken() for cases when ParseVariant() succeeds but returns a negative value?
Comment on attachment 51422 [details] [diff] [review] Patch to fix this The second patch fixes ParsePositiveVariant() so we handle this and negative paddings better and uses it...
Attachment #51422 - Attachment is obsolete: true
Comment on attachment 51483 [details] [diff] [review] Better patch r=dbaron
Attachment #51483 - Flags: review+
Comment on attachment 51483 [details] [diff] [review] Better patch sr=attinasi
Attachment #51483 - Flags: superreview+
Checked into trunk.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.