Bug 1504536 - Simplify the SVG animation code.
ClosedPublic
Actions

Authored by emilio on Nov 4 2018, 3:07 PM.

Details

Reviewers

hiro
birtles
heycam

Group Reviewers

Restricted Project

Commits

Restricted Diffusion Commit
rMOZILLACENTRAL6251012862a2: Bug 1504536 - Simplify the SVG animation code. r=hiro,heycam

Bugzilla Bug ID

1504536

Summary

It's overly generic for no good reason.

Diff Detail

Repository

rMOZILLACENTRAL mozilla-central

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

emilio created this revision.Nov 4 2018, 3:07 PM

Herald added a project: Restricted Project. · View Herald TranscriptNov 4 2018, 3:07 PM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 4 2018, 3:08 PM

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: Restricted Project.

Harbormaster completed remote builds in B19366: Diff 31594.Nov 4 2018, 3:08 PM

emilio requested review of this revision.Nov 4 2018, 3:08 PM

hiro added inline comments.Nov 4 2018, 11:43 PM

servo/components/style/values/animated/svg.rs
74–80	I am confused about this impl. Did I miss that the corresponding impl was moved from generics/svg.rs into this animated/svg.rs? Or the original impl should be removed in this patch? Or am I totally miss something?

Err, good catch, I need to remove https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/servo/components/style/values/generics/svg.rs#155 which I think on trunk panics if you specify calc, which seems bad.

I can also remove has_calc.

r=me on all the bits outside of values/animated/, which I defer to Hiro/Brian.

servo/components/style/properties/gecko.mako.rs
566	Nit: no comma at the end of braced match arms.

This revision is now accepted and ready to land.Nov 5 2018, 12:25 AM

In D10844#260875, @emilio wrote:

Err, good catch, I need to remove https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/servo/components/style/values/generics/svg.rs#155 which I think on trunk panics if you specify calc, which seems bad.

I can also remove has_calc.

r+ with this change.

Remove more dead code.

Harbormaster completed remote builds in B19443: Diff 31693.Nov 5 2018, 9:17 AM

emilio marked 3 inline comments as done.Nov 5 2018, 9:21 AM

emilio added inline comments.

servo/components/style/properties/gecko.mako.rs
566	As much as I agree with the sentiment, Servo's `rustfmt.toml` has the following: match_block_trailing_comma = true So given Servo has been rustfmt'ing stuff, I'd rather keep the style consistent.