'border-spacing' property in CSS2 cannot have negative lengths

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: Tete, Assigned: dbaron)

Tracking

({css2})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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
(Assignee)

Comment 3

16 years ago
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
Created attachment 51422 [details] [diff] [review]
Patch to fix this
reviews?
Keywords: css2, mozilla0.9.5, patch, review
(Assignee)

Comment 7

16 years ago
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?
Created attachment 51483 [details] [diff] [review]
Better patch
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
(Assignee)

Comment 12

16 years ago
Comment on attachment 51483 [details] [diff] [review]
Better patch

r=dbaron
Attachment #51483 - Flags: review+

Comment 13

16 years ago
Comment on attachment 51483 [details] [diff] [review]
Better patch

sr=attinasi
Attachment #51483 - Flags: superreview+
Checked into trunk.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.