Last Comment Bug 696918 - Restrict key input in DOM full-screen mode to explicit whitelist
: Restrict key input in DOM full-screen mode to explicit whitelist
Status: RESOLVED FIXED
[inbound]
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla10
Assigned To: Chris Pearce (:cpearce)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 545812
  Show dependency treegraph
 
Reported: 2011-10-24 15:27 PDT by Chris Pearce (:cpearce)
Modified: 2011-11-21 23:30 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (10.54 KB, patch)
2011-11-01 02:05 PDT, Chris Pearce (:cpearce)
bugs: review+
Details | Diff | Splinter Review

Description Chris Pearce (:cpearce) 2011-10-24 15:27:03 PDT
Currently key input in DOM full-screen mode is restricted to keys in the following ranges:

* DOM_VK_CANCEL to DOM_VK_CAPS_LOCK, inclusive
* DOM_VK_SPACE to DOM_VK_DELETE, inclusive
* DOM_VK_SEMICOLON to DOM_VK_EQUALS, inclusive
* DOM_VK_MULTIPLY to DOM_VK_META, inclusive

The security team wanted to be even more restrictive, and limit key input to an explicit whitelist of the following keys:

* tab, space, arrow keys, page-up, page-down, home, end,
* shift, ctrl, alt/option, command and combinations thereof, with the previous set of keys (except command/ctrl-tab).
Comment 1 David Chan [:dchan] 2011-10-25 10:26:11 PDT
List of keycodes for reference
http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/events/nsIDOMKeyEvent.idl#45


I would prefer the more restricted keyset unless there is a reason for allowing more keys than necessary. I'm assuming the goal is to provide the minimal number of keys that would allow the user to navigate/manipulate full screen controls.

Is the restriction of cmd/ctrl-tab to prevent the user from changing to another application? We should consider bug 685402 in the decision, though maybe we will just require the user to use the mouse for selecting a window in another monitor.
Comment 2 Chris Pearce (:cpearce) 2011-11-01 00:16:36 PDT
So the white-list so far proposed is:

DOM_VK_TAB
DOM_VK_SPACE
DOM_VK_PAGE_UP
DOM_VK_PAGE_DOWN
DOM_VK_END
DOM_VK_HOME
DOM_VK_LEFT
DOM_VK_UP
DOM_VK_RIGHT
DOM_VK_DOWN 
DOM_VK_SHIFT
DOM_VK_CONTROL
DOM_VK_ALT
DOM_VK_META

Do we want to include DOM_VK_RETURN and DOM_VK_ENTER?
Comment 3 Chris Pearce (:cpearce) 2011-11-01 02:05:39 PDT
Created attachment 570948 [details] [diff] [review]
Patch v1

Limit key input further. The only key codes which don't cause a "Press ESC to exit full-screen mode" warning to pop up when pressed are those listed in comment 2.
Comment 5 Marco Bonardo [::mak] 2011-11-03 08:41:34 PDT
https://hg.mozilla.org/mozilla-central/rev/ab1bb3e98ff9
Comment 6 Jean-Yves Perrier [:teoli] 2011-11-21 23:30:59 PST
This looks to be appropriately documented there:
https://developer.mozilla.org/en/DOM/Using_full-screen_mode#Things_your_users_want_to_know

I don't think it needs more documentation. If you disagree, re-flip the keyword, please.

Note You need to log in before you can comment on or make changes to this bug.