Closed Bug 520486 Opened 15 years ago Closed 15 years ago

Extend nsStyleAnimation & SVG/SMIL to support enumerated values

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files, 2 obsolete files)

nsStyleAnimation needs to be extended to support enumerated values (e.g. to support animating properties like "pointer-events" and "display")
OS: Linux → All
Hardware: x86 → All
Depends on: 522320
Summary: Extend nsStyleAnimation to support enumerated values → Extend nsStyleAnimation & SVG/SMIL to support enumerated values
This patch makes adds a new animated type "eStyleAnimType_Enum8" for enumerated values that are stored as PRUint8's in their style-struct.
Attachment #406388 - Flags: review?(dbaron)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attached patch attached wrong file (obsolete) (deleted) — Splinter Review
Here's "patch 1" again, rebased to apply on top of bug 522852 and bug 523193.
Attachment #407193 - Flags: review?(dbaron)
Attachment #406388 - Attachment is obsolete: true
Attachment #406388 - Flags: review?(dbaron)
Attached patch patch 1 (rebased) (deleted) — Splinter Review
(sorry, that last one was the wrong file -- reposting)
Attachment #407193 - Attachment is obsolete: true
Attachment #407194 - Flags: review?(dbaron)
Attachment #407193 - Flags: review?(dbaron)
Attachment #407193 - Attachment description: patch 1 (rebased) → attached wrong file
Comment on attachment 407194 [details] [diff] [review] patch 1 (rebased) Could you call it eStyleAnimType_EnumU8 since it's PRUint8? Also, in the comment for eStyleAnimType_EnumU8, you should comment that it requires that *all* of the enumerated values be accepted as an eCSSUnit_Enumerated value, which requires that they be in the kwtable_ entry in nsCSSPropList.h. (This also means that this patch depends on bug 522320 for some properties: display:none, font-style:normal, font-variant:normal, pointer-events:none, text-decoration:none, color-interpolation:auto, color-interpolation-filters:auto, dominant-baseline:auto, image-rendering:auto, shape-rendering:auto, text-rendering:auto. I'll need to check that all of those are fixed there.) Is it an issue that mTextDecoration is a bitfield rather than a set of values? Probably not. r=dbaron with that. Sorry for the delay: shouldn't have made you rebase it.
Attachment #407194 - Flags: review?(dbaron) → review+
(In reply to comment #5) > (From update of attachment 407194 [details] [diff] [review]) > Could you call it eStyleAnimType_EnumU8 since it's PRUint8? Fixed: http://hg.mozilla.org/users/dholbert_mozilla.com/smil-patches/rev/06010a0b17ca > Also, in the comment for eStyleAnimType_EnumU8, you should comment that > it requires that *all* of the enumerated values be accepted as an > eCSSUnit_Enumerated value, which requires that they be in the kwtable_ > entry in nsCSSPropList.h. Comment added: http://hg.mozilla.org/users/dholbert_mozilla.com/smil-patches/rev/37279e2814af > (This also means that this patch depends on bug 522320 Correct -- it depends on the first patch on that bug. (I'd thought I added a comment here to that effect, but I guess I just set the dependency field -- sorry if that wasn't clear.) > Is it an issue that mTextDecoration is a bitfield rather than a set of > values? Probably not. Nope, that's not an issue. We're just serializing the bitfield that we get from the style system (in ComputeValue/ExtractComputedValue), and then later deserializing it as a string (in UncomputeValue). The serialization is trivial ("copy the bitfield"). The deserialization is less trivial, but it's handled correctly by nsCSSDeclaration::AppendCSSValueToString (with a minor tweak, in bug 522320). > r=dbaron with that. Thanks for the review! I'll land this once the first patch on Bug 522320 is clear for landing. > Sorry for the delay: shouldn't have made you rebase it. No problem -- as rebases go, this one was easy. :) (mostly taken care of by a simple s/eStyleUnit_/eUnit_/ in the patch file.)
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Depends on: 525099
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: