Closed Bug 1354053 Opened 3 years ago Closed 3 years ago

stylo: Make word-spacing animatable

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

Details

Attachments

(1 file, 3 obsolete files)

Initially I was not going to implement this by myself, but Boris told me that word-spacing is used in test_transitions.html,  so yeah, I am going to implement this now.
Comment on attachment 8855257 [details]
Bug 1354053 - Make word-spacing animatable.

https://reviewboard.mozilla.org/r/127128/#review129882

::: servo/components/style/properties/gecko.mako.rs:2914
(Diff revision 1)
> +                     CoordDataValue::Normal |
> +                     CoordDataValue::Coord(_) |
> +                     CoordDataValue::Percent(_) |
> +                     CoordDataValue::Calc(_)),
> +            "Unexpected computed value for word-spacing");
> +        T(LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing))

When I made letter-spacing animatable, I did not notice this from_gecko_style_coord() method. 
In the subsequent patch, I did also rewrite close_letter_spacing() with this function.
Comment on attachment 8855258 [details]
Bug 1354053 - Use from_gecko_style_corrd() to convert CoordDataValue to Option<T> for letter-spacing.

https://reviewboard.mozilla.org/r/127130/#review130044
Attachment #8855258 - Flags: review?(emilio+bugs) → review+
Comment on attachment 8855256 [details]
Bug 1354053 - Introduce a macro to define Interpolate trait for tuple struct which has Option<T>.

https://reviewboard.mozilla.org/r/127126/#review130046
Attachment #8855256 - Flags: review?(emilio+bugs) → review+
Comment on attachment 8855257 [details]
Bug 1354053 - Make word-spacing animatable.

https://reviewboard.mozilla.org/r/127128/#review129894

::: servo/components/style/properties/gecko.mako.rs:2914
(Diff revision 1)
> +                     CoordDataValue::Normal |
> +                     CoordDataValue::Coord(_) |
> +                     CoordDataValue::Percent(_) |
> +                     CoordDataValue::Calc(_)),
> +            "Unexpected computed value for word-spacing");
> +        T(LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing))

Sounds good, thanks!
Attachment #8855257 - Flags: review?(emilio+bugs) → review+
Comment on attachment 8855259 [details]
Bug 1354053 - Enable word-spacing interpolation test.

https://reviewboard.mozilla.org/r/127132/#review130108
Attachment #8855259 - Flags: review?(bbirtles) → review+
Comment on attachment 8855258 [details]
Bug 1354053 - Use from_gecko_style_corrd() to convert CoordDataValue to Option<T> for letter-spacing.

https://reviewboard.mozilla.org/r/127130/#review130110

::: commit-message-657df:1
(Diff revision 1)
> +Bug 1354053 - Use from_gecko_style_corrd() to convert CoordDataValue to Option<T> for letter-spacing. r?emilio

s/from_gecko_style_corrd/from_gecko_style_coord/
Attachment #8855256 - Attachment is obsolete: true
Attachment #8855257 - Attachment is obsolete: true
Attachment #8855258 - Attachment is obsolete: true
Thanks for the review. I will send a PR once https://github.com/servo/servo/pull/16292 gets merged since it conflicts with this changes.
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c7215aa1bfe
Enable word-spacing interpolation test. r=birtles
https://hg.mozilla.org/mozilla-central/rev/1c7215aa1bfe
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.