Opened 3 years ago

Closed 19 months ago

#16337 closed task (fixed)

Investigate whether Animations(Player) API provides new high resolution timestamp

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff52-esr, tbb-fingerprinting-time-highres, tbb-7.0-must-alpha, TorBrowserTeam201705R
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor4

Description (last modified by gk)

The Animation(Player) API got implemented (https://developer.mozilla.org/en-US/docs/Web/API/Animation) and may provide a new high resolution timestamp. If so, make sure to reduce its granularity respectively. This is behind a pref, dom.animations-api.core.enabled in Firefox 38 ESR. Thus, deferring to Firefox 45 ESR.

Child Tickets

Change History (22)

comment:1 Changed 3 years ago by gk

Keywords: tbb-fingerprinting-time-highres added; tbb-fingerprinting-high-res removed

comment:2 Changed 3 years ago by gk

Description: modified (diff)
Summary: Investigate whether Animations API provides new high resolution timestampInvestigate whether Animations(Player) API provides new high resolution timestamp

comment:3 Changed 3 years ago by gk

Severity: Normal
Sponsor: None

Still preffed off -> ff52-esr

comment:4 Changed 3 years ago by bugzilla

Keywords: ff52-esr added; ff45-esr removed

comment:5 Changed 21 months ago by gk

Keywords: ff59-esr added; ff52-esr removed

Still disabled for esr52.

comment:6 Changed 21 months ago by gk

Keywords: ff52-esr added; ff59-esr removed
Sponsor: NoneSponsor4

Actually, Element.animate() seems to be available right now already even if the other features of the API are not implemented yet (or not available on the release channel). See: https://bugzilla.mozilla.org/show_bug.cgi?id=1245000. Moving this back on our ESR 52 radar.

comment:7 Changed 21 months ago by cypherpunks

comment:8 Changed 19 months ago by gk

Keywords: tbb-7.0-must added

More tickets for 7.0.

comment:9 Changed 19 months ago by gk

Keywords: tbb-7.0-must-alpha added; tbb-7.0-must removed

Getting more tickets on our alpha radar.

comment:10 Changed 19 months ago by gk

Priority: MediumHigh

Moving the investigation tickets to higher priority.

comment:11 Changed 19 months ago by gk

dom.animations-api.element-animate.enabled is the relevant preference here.

comment:12 Changed 19 months ago by gk

Keywords: TorBrowserTeam201705 added

Moving more tickets on our May 2015 radar.

comment:13 Changed 19 months ago by arthuredelstein

Keywords: TorBrowserTeam201705R added; TorBrowserTeam201705 removed
Status: newneeds_review

comment:14 Changed 19 months ago by tom

Does this need to get added to the in-progress Mozilla patch?

comment:15 in reply to:  13 ; Changed 19 months ago by gk

Replying to arthuredelstein:

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/16337

We don't want to have a ResistFingerprinting-check here? What makes that feature different from, say, the one in #10286 where you explicitly added one? Or maybe that's just because the other timestamp clamping code does not have such a check either?

comment:16 in reply to:  14 Changed 19 months ago by arthuredelstein

Replying to tom:

Does this need to get added to the in-progress Mozilla patch?

Yes, I'll add it tomorrow.

comment:17 in reply to:  15 Changed 19 months ago by arthuredelstein

Replying to gk:

Replying to arthuredelstein:

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/16337

We don't want to have a ResistFingerprinting-check here? What makes that feature different from, say, the one in #10286 where you explicitly added one? Or maybe that's just because the other timestamp clamping code does not have such a check either?

Yes, I left it out to be consistent with the other clamping code we currently have in Tor Browser. Once it gets uplifted to Mozilla, we can put it behind a pref. Jonathan has already created infrastructure for that in https://bugzilla.mozilla.org/show_bug.cgi?id=1217238

comment:18 in reply to:  13 ; Changed 19 months ago by mcs

Replying to arthuredelstein:

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/16337

This patch looks okay. Do we also need to patch GetCurrentTimeAsDouble() inside dom/animation/AnimationTimeline.h? Kathy and I are not sure how the Animation.timeline object relates to the other timing information.

comment:19 in reply to:  18 Changed 19 months ago by arthuredelstein

Replying to mcs:

Replying to arthuredelstein:

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/16337

This patch looks okay. Do we also need to patch GetCurrentTimeAsDouble() inside dom/animation/AnimationTimeline.h? Kathy and I are not sure how the Animation.timeline object relates to the other timing information.

That's a good point. Animation.timeline and document.timeline aren't exposed in TBB/ESR52 because "dom.animations-api.core.enabled" is set to false. But I looked at it again and I found there is a simpler way to implement this patch that covers the *.timeline cases as well. So here's an updated patch. (The document.timeline.currentTime test doesn't run unless the "dom.animations-api.core.enabled" pref has been set to true by default, but I don't think this is a problem for TBB.)

https://github.com/arthuredelstein/tor-browser/commit/16337+1

comment:20 Changed 19 months ago by gk

Thanks! Looks good to me.

comment:21 Changed 19 months ago by mcs

r=brade, r=mcs
This looks good to us too.

comment:22 Changed 19 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

This is commit 77f0de013fa2b5bedb851507f5ec94a8f39f8b8c on tor-browser-52.1.0esr-7.0-2 now.

Note: See TracTickets for help on using tickets.