Closed
Bug 1454524
Opened 6 years ago
Closed 6 years ago
Remove some unused comparator code in KeyframeUtils
Categories
(Core :: DOM: Animation, enhancement)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
Details
Attachments
(1 file)
No description provided.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•6 years 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.)
Comment 3•6 years ago
|
||
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.
Comment 4•6 years ago
|
||
(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.
Comment 5•6 years ago
|
||
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
Comment 6•6 years ago
|
||
Thanks. I had actually got involved in that bug (bug 1371493 comment 5), but don't recall. :)
Comment 7•6 years 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+
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•6 years 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•6 years ago
|
||
But that's not a blocker anyway.
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/70235b1de25d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•