If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add some more values to property_database.js

RESOLVED FIXED in mozilla21

Status

()

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

People

(Reporter: Jesse Ruderman, Assigned: Jesse Ruderman)

Tracking

Trunk
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 712782 [details] [diff] [review]
patch
Attachment #712782 - Flags: review?(dbaron)
(Assignee)

Comment 1

5 years ago
Created attachment 712783 [details]
i like snakes
(Assignee)

Comment 2

5 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 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+
(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

5 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

5 years ago
Created attachment 713787 [details] [diff] [review]
patch v2
Attachment #712782 - Attachment is obsolete: true
Attachment #713787 - Flags: review?(dbaron)
Comment on attachment 713787 [details] [diff] [review]
patch v2

r=dbaron; thanks for doing this
Attachment #713787 - Flags: review?(dbaron) → review+
(Assignee)

Comment 8

5 years ago
I did it for my DOM fuzzer ;)

https://hg.mozilla.org/integration/mozilla-inbound/rev/71f76fd4fbab
https://hg.mozilla.org/mozilla-central/rev/71f76fd4fbab
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.