KeyboardEvents are only rounding to 100ms

In #1517 (moved), the idea was to round most event time stamps to 100ms, but to round KeyboardEvent.timeStamp to 250 ms (see https://trac.torproject.org/projects/tor/ticket/1517#comment:25).

Our current patch for #1517 (moved) is here.

However, when I test KeyboardEvents in the latest Tor Browser, I only see rounding to 100 ms. With the debugger I observed that calling myKeyboardEvent.timeStamp in the browser console is calling Event::TimeStamp() instead of KeyboardEvent::TimeStamp. So we're apparently failing to properly override the Event::TimeStamp call.

added TorBrowserTeam201606R component::applications/tor browser owner::arthuredelstein priority::medium resolution::fixed severity::normal status::closed tbb-fingerprinting type::defect labels

If we want to make sure that KeyboardEvent::timeStamp rounding to 250 ms is exposed to content JS, we can make Event::timeStamp virtual (and fix the return type of KeyboardEvent::timeStamp). Here's an example fixup, probably not for use.

But I'm not sure this is really useful, as it's still very easy to get 100-ms precision on keystroke times. Just ignore KeyboardEvent.timeStamp and use Date.now() on the KeyboardEvent handler:

document.addEventListener("keydown", e => console.log(e.key + ": " + Date.now()));

So I'm proposing the following patch instead, which simply removes our original change to KeyboardEvent.h. Then we would rely on the 100-ms rounding inside Event.cpp, which is already applied to KeyboardEvents. (If it looks like 100-ms is unsufficient jitter for KeyboardEvents, then we may want to increase the rounding for all JS timing sources.)

https://github.com/arthuredelstein/tor-browser/commit/19186 Hash cb8eace4cd33cc9905bb7edbb8d9c066cc4217df

Trac:
Keywords: TorBrowserTeam201505 deleted, TorBrowserTeam201505R added
Status: new to needs_review

Dragging this into 2016 :)

Trac:
Keywords: TorBrowserTeam201505R deleted, TorBrowserTeam201605R added

Is that a regression due to changes in ESR 45 or did we already have this problem with ESR38-bases Tor Browsers?

Replying to gk:

Is that a regression due to changes in ESR 45 or did we already have this problem with ESR38-bases Tor Browsers?

Turns out it was already there -- I see this problem in Tor Browser 5.5.

Looks good to me. And if that has always been as your patch proposes then we probably won't have a ton of users coming to us complaining about broken websites/games/whatever.

However, it bothers me a bit that we did not catch that earlier. Could we maybe have a test for that (a new ticket for it is fine)?

Replying to gk:

Looks good to me. And if that has always been as your patch proposes then we probably won't have a ton of users coming to us complaining about broken websites/games/whatever.

However, it bothers me a bit that we did not catch that earlier. Could we maybe have a test for that (a new ticket for it is fine)?

Agreed. I posted regression tests at ticket:19193#comment:1.

Mark/Kathy: Could you have a look at this one, too?

Trac:
Cc: mikeperry to mikeperry, mcs, brade

Moving reviews to June

Trac:
Keywords: N/A deleted, TorBrowserTeam201606 added

Trac:
Keywords: TorBrowserTeam201605R deleted, TorBrowserTeam201606R added

Trac:
Keywords: TorBrowserTeam201606 deleted, N/A added

Replying to gk:

Mark/Kathy: Could you have a look at this one, too?

r=brade, r=mcs This looks fine to us as well.

Commit 7cfce1d2cd71ed69ae3ffb4e59007eeae5ac5815 on tor-browser-45.2.0esr-6.5-1 has it, thanks.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue #20680 (moved)

mentioned in issue #20680 (moved)

moved to tpo/applications/tor-browser#19186 (closed)