Closed Bug 1078119 Opened 5 years ago Closed 5 years ago
Player::Get Current Time Duration
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
Forgot to drop a declaration of GetCurrentTimeDuration
Attachment #8500257 - 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+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.