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

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
CSS Parsing and Computation
P4
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla1.9.2a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

9 years ago
I noticed a bunch of problems with serialization of content:counter() and counters() functions.  Patches to follow.
(Assignee)

Comment 1

9 years ago
Note that these patches apply on top of the patches in bug 478160.
(Assignee)

Comment 2

9 years ago
Created 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())

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)
(Assignee)

Comment 3

9 years ago
Created attachment 365617 [details] [diff] [review]
patch 2: fix computed style serialization to output keywords rather than integers
Attachment #365617 - Flags: superreview?(bzbarsky)
Attachment #365617 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

9 years ago
Created attachment 365618 [details] [diff] [review]
patch 3: fix serialization of declarations whose list-style-type part is none
Attachment #365618 - Flags: superreview?(bzbarsky)
Attachment #365618 - Flags: review?(bzbarsky)
(Assignee)

Updated

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

Updated

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

Comment 5

9 years ago
Created attachment 365619 [details] [diff] [review]
patch 4: tests

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+
(Assignee)

Comment 7

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

Comment 8

9 years ago
http://hg.mozilla.org/mozilla-central/rev/3428afb4e233
http://hg.mozilla.org/mozilla-central/rev/3b13a87771fe
http://hg.mozilla.org/mozilla-central/rev/36da7fe92b5f
http://hg.mozilla.org/mozilla-central/rev/ac91e21d53c5
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Priority: -- → P4
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.