Closed Bug 1078119 Opened 5 years ago Closed 5 years ago

Rename AnimationPlayer::GetCurrentTimeDuration

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: birtles, Assigned: birtles)

References

Details

Attachments

(1 file, 1 obsolete file)

Since I discovered BinaryName in WebIDL I'm a bit unsure how much to use it in the Web Animations work.

There are a few cases where we have functions which we use internally and from the DOM but where they behave differently.

In bug 1037321 we have both:

  PlayState()
  PlayStateFromJS()

The latter flushes style first. However, we often want to query to play state internally so it seems to make sense to give the nice name to the one intended for internal use rather than risk people accidentally flushing style.

I was thinking of doing the same thing for methods that get/set times. Internally we work with TimeDuration objects but for DOM callers we convert this to a DOMHighResTimeStamp (a double).

So I was thinking we should have:

  Nullable<TimeDuration> GetCurrentTime()
  Nullable<double>       GetCurrentTimeAsDouble()

We fetch the current time a lot internally so this is nicer than having to write GetCurrentTimeAsDuration() or something similar.

The only problem I can see is that nearly every method in the public API will end up being renamed for one of the above reasons. I'm not sure if this is a problem or not.
This patch performs the following renaming:

  AnimationPlayer::GetCurrentTime -> GetCurrentTimeAsDouble
  AnimationPlayer::GetCurrentTimeDuration -> GetCurrentTime
  AnimationTimeline::GetCurrentTime -> GetCurrentTimeAsDouble
  AnimationTimeline::GetCurrentTimeDuration -> GetCurrentTime
Attachment #8500256 - Flags: review?(bzbarsky)
Blocks: 1078122
Forgot to drop a declaration of GetCurrentTimeDuration
Attachment #8500257 - Flags: review?(bzbarsky)
Attachment #8500256 - Attachment is obsolete: true
Attachment #8500256 - Flags: review?(bzbarsky)
Comment on attachment 8500257 [details] [diff] [review]
Rename AnimationTimeline/AnimationPlayer GetCurrentTimeDuration

r=me

In this case another option would have been to return some class with auto-conversions to both double and TimeDuration (e.g. a subclass of TimeDuration with a conversion operator to double).  But that seems a lot more confusing than just having separate methods.
Attachment #8500257 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/f6866bdaa73d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.