diff options
| -rw-r--r-- | content_scripts/mode.coffee | 20 | ||||
| -rw-r--r-- | content_scripts/mode_find.coffee | 9 | ||||
| -rw-r--r-- | content_scripts/mode_insert.coffee | 28 | ||||
| -rw-r--r-- | lib/dom_utils.coffee | 3 |
4 files changed, 47 insertions, 13 deletions
diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index a33197b0..5c6201e0 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -111,23 +111,21 @@ class Mode @passKeys = passKeys @registerStateChange?() - # If @options.trapAllKeyboardEvents is truthy, then it should be an element. All keyboard events on that - # element are suppressed *after* bubbling the event down the handler stack. This prevents such events - # from propagating to other extensions or the host page. - if @options.trapAllKeyboardEvents + # If @options.suppressPrintableEvents is truthy, then it should be an element. All printable keyboard + # events on that element are suppressed, if necessary (that is, *after* bubbling down the handler stack). + # We only suppress keypress events. This is used by PostFindMode to protect active, editable elements. + # Note: We use unshift here, not push, so the handler is installed at the bottom of the stack. + if @options.suppressPrintableEvents @unshift - _name: "mode-#{@id}/trapAllKeyboardEvents" - keydown: (event) => - if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling + _name: "mode-#{@id}/suppressPrintableEvents" keypress: (event) => - if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling - keyup: (event) => - if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling + if DomUtils.isPrintable(event) and + event.srcElement == @options.suppressPrintableEvents then @suppressEvent else @continueBubbling Mode.updateBadge() if @badge Mode.modes.push @ @log() if @debug - handlerStack.debugOn() + # handlerStack.debugOn() # End of Mode constructor. push: (handlers) -> diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee index 740358d5..91ae4507 100644 --- a/content_scripts/mode_find.coffee +++ b/content_scripts/mode_find.coffee @@ -1,10 +1,14 @@ # NOTE(smblott). Ultimately, all of the FindMode-related code should be moved to this file. # When we use find mode, the selection/focus can end up in a focusable/editable element. In this situation, -# special considerations apply. We implement two special cases: +# special considerations apply. We implement three special cases: # 1. Prevent keyboard events from dropping us unintentionally into insert mode. This is achieved by # inheriting from InsertModeBlocker. -# 2. If the very-next keystroke is Escape, then drop immediately into insert mode. +# 2. Prevent all printable keyboard events on the active element from propagating. This is achieved by setting the +# suppressPrintableEvents option. There's some controversy as to whether this is the right thing to do. +# See discussion in #1415. This implements Option 2 from there, although Option 3 would be a reasonable +# alternative. +# 3. If the very-next keystroke is Escape, then drop immediately into insert mode. # class PostFindMode extends InsertModeBlocker constructor: (findModeAnchorNode) -> @@ -16,6 +20,7 @@ class PostFindMode extends InsertModeBlocker # instance is automatically deactivated when a new instance is activated. singleton: PostFindMode exitOnBlur: element + suppressPrintableEvents: element return @exit() unless element and findModeAnchorNode diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 5720c901..b907f22e 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -96,6 +96,34 @@ class InsertModeBlocker extends Mode new @options.onClickMode targetElement: document.activeElement +# There's an unfortunate feature interaction between chrome's contentEditable handling and our insert mode. +# If the selection is contentEditable and a descendant of the active element, then chrome focuses it on any +# unsuppressed printable keypress. This drops us unintentally into insert mode. See #1415. A single +# instance of this mode sits near the bottom of the handler stack and suppresses each keypress event if: +# - it hasn't been handled by any other mode (so not by normal mode, passkeys, insert, ...), +# - it represents a printable character, +# - the selection is content editable, and +# - the selection is a descendant of the active element. +# This should rarely fire, typically only on fudged keypresses in normal mode. And, even then, only in the +# circumstances outlined above. So, we shouldn't usually be blocking keyboard events for other extensions or +# the page itself. +# There's some controversy as to whether this is the right thing to do. See discussion in #1415. This +# implements Option 2 from there. +new class ContentEditableTrap extends Mode + constructor: -> + super + name: "content-editable-trap" + keypress: (event) => + if @wouldTriggerInsert event then @suppressEvent else @continueBubbling + + # True if the selection is content editable and a descendant of the active element. + wouldTriggerInsert: (event) -> + element = document.getSelection()?.anchorNode?.parentElement + return element?.isContentEditable and + document.activeElement and + DomUtils. isPrintable event and + DomUtils.isDOMDescendant document.activeElement, element + root = exports ? window root.InsertMode = InsertMode root.InsertModeTrigger = InsertModeTrigger diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 322188b3..fd2427c4 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -212,5 +212,8 @@ DomUtils = @remove() false + isPrintable: (event) -> + not (event.metaKey or event.ctrlKey or event.altKey) + root = exports ? window root.DomUtils = DomUtils |