unit tests for keyboard defenses

Our patches for legacy/trac#15646 (moved) and legacy/trac#17009 (moved) should have unit tests.

added TorBrowserTeam201605R in Legacy / Trac component::applications/tor browser in Legacy / Trac owner::tbb-team in Legacy / Trac priority::medium in Legacy / Trac resolution::fixed in Legacy / Trac severity::normal in Legacy / Trac status::closed in Legacy / Trac type::defect in Legacy / Trac labels

Here is a patch for review. Please note there are two patches -- regression tests for 15646, and regression tests for 17009:

https://github.com/arthuredelstein/tor-browser/commits/17790.

Trac:
Keywords: N/A deleted, TorBrowserTeam201512R added

Trac:
Status: new to needs_review

Carrying over reviews.

Trac:
Keywords: TorBrowserTeam201512R deleted, TorBrowserTeam201601R added

e58782ab2034cf30ea548b0f87de122ac4805118

+        // We should only see the modifier key in content if suppression is
+        // active; otherwise we expect to see the "x" key instead.
+        let expectedContentKey = suppressModifiers ? "x" : modifierKey;

It seems the comment does not match the code? We get "x" if the modifiers are suppressed and the modifier key otherwise (which is intended). So, s/active/not active/ ?

More importantly, the behavior of privacy.suppressModifierKeyEvents is dependent on the value for privacy.resistFingerprinting but the test does not take that into account. I think we should at least assume explicitly that the latter is true. It might be good, though, to test as well with the latter being false to make sure the code for legacy/trac#17009 (moved) is not kicking in even if privacy.suppressModifierKeyEvents is true.

d5481537329a7a77ab597f40892c7df83d0ffcc2

+  // Return a promise that resolves to the event when]

s/]//

legacy/trac#15646 (moved) takes care of more things than keyCode and shiftKey, e.g: code and loaction as well and Alt-key handling. What is with those?

Trac:
Status: needs_review to needs_revision

Replying to gk:

[...]

All good comments and suggestions. I have made fixes to both patches to try to address all of these. Also, in writing the additional tests, I found a mistake in the KeyboardEvent patch, so I am including a fixup patch in this new branch:

https://github.com/arthuredelstein/tor-browser/commits/17790+1

There are three patches in total.

Replying to arthuredelstein:

Replying to gk:

[...]

All good comments and suggestions. I have made fixes to both patches to try to address all of these. Also, in writing the additional tests, I found a mistake in the KeyboardEvent patch, so I am including a fixup patch in this new branch:

https://github.com/arthuredelstein/tor-browser/commits/17790+1

There are three patches in total.

I've taken the fixup for tbb-5.5. The tests need a closer look for which I don't have the time now.

Trac:
Status: needs_revision to needs_review

Carrying over review tickets.

Trac:
Keywords: TorBrowserTeam201601R deleted, TorBrowserTeam201602R added

Trac:
Keywords: TorBrowserTeam201602R deleted, TorBrowserTeam201603R added

No easy way to do reviews in March anymore.

Trac:
Keywords: TorBrowserTeam201603R deleted, TorBrowserTeam201604R added

Moving reviews over to May 2016

Trac:
Keywords: TorBrowserTeam201604R deleted, TorBrowserTeam201605R added

I finally got to look at the tests again. I cherry-picked both commits to tor-browser-45.1.0esr-6.0-1 (commit b815400e and de5346ad), thanks.

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

closed

moved from legacy/trac#17790 (moved)

added Bug label and removed 1 deleted label

removed 1 deleted label