Closed
Bug 840402
Opened 10 years ago
Closed 10 years ago
Add some more values to property_database.js
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: jruderman, Assigned: jruderman)
Details
Attachments
(2 files, 1 obsolete file)
1.46 KB,
text/plain
|
Details | |
8.31 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #712782 -
Flags: review?(dbaron)
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
I also tried to add these, but they caused failures: not serialized this way? overflow: -moz-scrollbars-none, -moz-scrollbars-horizontal, -moz-scrollbars-vertical relative things: font-stretch: narrower | wider font-size: larger | smaller
Comment 3•10 years ago
|
||
Comment on attachment 712782 [details] [diff] [review] patch Maybe leave the existing last invalid_values and add your new one (background-color)? Your final initial_values entry for color is wrong; please remove it. It's a bug that we clamp rgb() to 0-255. -moz-use-system-font as an initial value for 'font-family' is also kinda wrong; it only comes out initial when there's no system font in the 'font' shorthand, so I wouldn't include that. All those new other_values for font-family are pretty weird -- you should at least have a code comment explaining that for font-family (unlike font) they're not special values, they're just identifiers -- and maybe use some that aren't -moz-* r=dbaron with those things fixed
Attachment #712782 -
Flags: review?(dbaron) → review+
Comment 4•10 years ago
|
||
(In reply to Jesse Ruderman from comment #2) > I also tried to add these, but they caused failures: > > not serialized this way? > overflow: -moz-scrollbars-none, -moz-scrollbars-horizontal, > -moz-scrollbars-vertical -moz-scrollbars-none ought towork (synonym for hidden), and the other two also ought to work once we implement two-value 'overflow'. What failed? > relative things: > font-stretch: narrower | wider We don't implement these, and they were dropped from the spec. Could test them in invalid_values, though. > font-size: larger | smaller I'd think they'd have worked as other_values. What failed?
Assignee | ||
Comment 5•10 years ago
|
||
> Maybe leave the existing last invalid_values and add your new one > (background-color)? The new test is stronger, since there's only one reason to reject the input. > Your final initial_values entry for color is wrong; please remove it. It's a bug > that we clamp rgb() to 0-255. I see. I'll leave it out, and let you add a more correct test in bug 515919. > -moz-use-system-font as an initial value for 'font-family' is also kinda wrong; > it only comes out initial when there's no system font in the 'font' shorthand, so > I wouldn't include that. When there is *not* a system font? Anyway, this doesn't seem to fit in property_database.js, so I'll teach my fuzzer about it instead. > All those new other_values for font-family are pretty weird -- you should at > least have a code comment explaining that for font-family (unlike font) they're > not special values Oops, they should have gone in font rather than font-family. > ... they're just identifiers -- and maybe use some that aren't -moz-* I'll stick "-no-such-font-installed" in font-family as the "just an identifier" test. >> overflow: -moz-scrollbars-none, -moz-scrollbars-horizontal, >> -moz-scrollbars-vertical > -moz-scrollbars-none ought to work (synonym for hidden), and the other two also > ought to work once we implement two-value 'overflow'. What failed? You're right, -moz-scrollbars-none does okay. I'll just add that one for now. >> font-stretch: narrower | wider > We don't implement these, and they were dropped from the spec. That would explain it! >> font-size: larger | smaller > I'd think they'd have worked as other_values. What failed? Yeah, I assumed incorrectly based on the font-stretch results. Added these.
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #712782 -
Attachment is obsolete: true
Attachment #713787 -
Flags: review?(dbaron)
Comment 7•10 years ago
|
||
Comment on attachment 713787 [details] [diff] [review] patch v2 r=dbaron; thanks for doing this
Attachment #713787 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 8•10 years ago
|
||
I did it for my DOM fuzzer ;) https://hg.mozilla.org/integration/mozilla-inbound/rev/71f76fd4fbab
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/71f76fd4fbab
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•