Remove some unused comparator code in KeyframeUtils

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

Trunk
mozilla61
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
(Came across when investigating usage of some tables in nsCSSProps. Unfortunately it isn't going to help removing any bits of code there at the end, though.)
Nice catch!  I haven't noticed that it's not used any more.  Though it's not used, I am not convinced whether we can really remove it.  Especially sorting shorthand by sub property count.  IIRC, we don't have such automation test cases and probably it's not yet spec-ed.  Let me take time to check our code.

CCing Brian, he might know something about it.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

> probably it's not yet spec-ed.

Great!  It's already spec-ed.  From https://drafts.csswg.org/web-animations-1/#calculating-computed-keyframes

> 2. Shorthand properties with fewer longhand components override those with more longhand components (e.g. border-top overrides border-color).

So there must be some test cases in web platform tests.
Yeah, I rewrote the same code in rust as part of Stylo.[1] I think we don't need this once dropping Gecko style system.

[1] https://github.com/servo/servo/pull/17831/files
Thanks.  I had actually got involved in that bug (bug 1371493 comment 5), but don't recall. :)

Comment 7

a year ago
mozreview-review
Comment on attachment 8968371 [details]
Bug 1454524 - Remove some unused comparator code in KeyframeUtils.

https://reviewboard.mozilla.org/r/237056/#review242888
Attachment #8968371 - Flags: review?(hikezoe) → review+

Comment 8

a year ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70235b1de25d
Remove some unused comparator code in KeyframeUtils. r=hiro
(Assignee)

Comment 9

a year ago
I'm wondering whether it is possible to remove other uses of nsCSSProps::PropertyIDLName() and nsCSSProps::PropertyIDLNameSortPosition(). There seems to be some remaining in animation code.
(Assignee)

Comment 10

a year ago
But that's not a blocker anyway.
https://hg.mozilla.org/mozilla-central/rev/70235b1de25d
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.