Closed Bug 481591 Opened 11 years ago Closed 11 years ago

fix problems with content: counter() and counters() serialization

Categories

(Core :: CSS Parsing and Computation, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: dbaron, Assigned: dbaron)

Details

Attachments

(4 files)

I noticed a bunch of problems with serialization of content:counter() and counters() functions.  Patches to follow.
Note that these patches apply on top of the patches in bug 478160.
This splits eCSSUnit_String into String (quoted), Ident (unquoted), and Families (font-family list).  Note that the font-family descriptor is String rather than Families.

(I'd like to eventually change Families into some other data type; there's even an existing bug on that.)
Attachment #365616 - Flags: superreview?(bzbarsky)
Attachment #365616 - Flags: review?(bzbarsky)
Attachment #365616 - Attachment description: patch 1: split eCSSUnit_String into String, Ident, and Families → patch 1: split eCSSUnit_String into String, Ident, and Families (which gives us quotes around the separator-part of counters())
Attachment #365617 - Attachment description: patch 2: serialization bug 1, fix computed style to output keywords rather than integers → patch 2: fix computed style serialization to output keywords rather than integers
Attached patch patch 4: testsSplinter Review
I wrote these tests while looking at the code coverage output in http://people.mozilla.org/~mnandigama/codecoverage/src/layout/style/nsCSSDeclaration.cpp.gcov.html , which led me to the bugs fixed here.
Attachment #365616 - Flags: superreview?(bzbarsky)
Attachment #365616 - Flags: superreview+
Attachment #365616 - Flags: review?(bzbarsky)
Attachment #365616 - Flags: review+
Comment on attachment 365616 [details] [diff] [review]
patch 1: split eCSSUnit_String into String, Ident, and Families (which gives us quotes around the separator-part of counters())

Can we add an assert on the unit in the nsCSSFontFaceStyleDecl::GetPropertyValue eCSSFonDesc_Family case, where the comment talks about knowing the unit?

Do we still need to Trim() the quote from the eCSSFontDesc_Family in InsertFontFaceRule?

With the assert added and the Trim() checked, looks good.
Attachment #365617 - Flags: superreview?(bzbarsky)
Attachment #365617 - Flags: superreview+
Attachment #365617 - Flags: review?(bzbarsky)
Attachment #365617 - Flags: review+
Attachment #365618 - Flags: superreview?(bzbarsky)
Attachment #365618 - Flags: superreview+
Attachment #365618 - Flags: review?(bzbarsky)
Attachment #365618 - Flags: review+
(In reply to comment #6)
> Do we still need to Trim() the quote from the eCSSFontDesc_Family in
> InsertFontFaceRule?

No, and I don't think we did before this patch either.
You need to log in before you can comment on or make changes to this bug.