If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Clean up AnimationPlayerCollection::PseudoElement()

RESOLVED FIXED in mozilla36

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla36
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Right now, AnimationPlayerCollection::PseudoElement() looks like this:

254   nsString PseudoElement()
255   {
256     if (IsForElement()) {
257       return EmptyString();
258     } else if (IsForBeforePseudo()) {
259       return NS_LITERAL_STRING("::before");
260     } else {
261       return NS_LITERAL_STRING("::after");
262     }
263   }

http://mxr.mozilla.org/mozilla-central/source/layout/style/AnimationCommon.h?rev=a014629454d0#254

Three things that I think should change:
 (1) It should be marked as "const"
 (2) We should drop "else" after return
 (3) In the third case, we should probably assert that "IsForAfterPseudo()" is true, to sanity-check that we don't accidentally fall into that final "return" for a non-::after scenario.
(Assignee)

Comment 1

3 years ago
Created attachment 8506406 [details] [diff] [review]
fix v1
Attachment #8506406 - Flags: review?(birtles)
Attachment #8506406 - Flags: review?(birtles) → review+
(Assignee)

Comment 2

3 years ago
Thanks! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/375f9239e689
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/375f9239e689
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.