Cleanup a bit nsDOMCSSAttributeDeclaration.

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

In preparation of removing the serialization roundtrip that SMIL does, which is blocking bug 1457353.

After this we can use Uncompute instead of nsSMILCSSValueType in order to avoid trying to serialize complex colors.
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8972512 [details]
Bug 1458458: Cleanup a bit nsDOMCSSAttrDeclaration.

https://reviewboard.mozilla.org/r/241100/#review246886

Looks good to me basically but we should split dropping the code for the old style stuff into a separate patch.

::: dom/smil/nsSMILCSSProperty.cpp:110
(Diff revision 1)
>  
>  nsresult
>  nsSMILCSSProperty::SetAnimValue(const nsSMILValue& aValue)
>  {
>    NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID), NS_ERROR_FAILURE);
> -
> +  return mElement->GetSMILOverrideStyle()->SetSMILValue(mPropID, aValue);

I am not concinved we can ensure that GetSMILOverriderStyle() doesn't return nullptr here.  Are you?
Attachment #8972512 - Flags: review?(hikezoe) → review+
(Assignee)

Comment 3

a year ago
It creates the declaration if it doesn't exist, so yeah, can't return null:

  https://searchfox.org/mozilla-central/rev/795575287259a370d00518098472eaa5b362bfa7/dom/base/Element.cpp#2051
(Assignee)

Updated

a year ago
Blocks: 1458580

Comment 4

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b54012d35b8d
Remove dead code from nsDOMCSSAttributeDeclaration. r=hiro
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ee81dec09bd
Minor nsDOMCSSAttrDeclaration cleanup. r=hiro
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb5befa3fdba
Move setting SMIL override values into nsDOMCSSAttrDeclaration. r=hiro

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b54012d35b8d
https://hg.mozilla.org/mozilla-central/rev/8ee81dec09bd
https://hg.mozilla.org/mozilla-central/rev/bb5befa3fdba
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.