From dc423eff18b2b2654c175633cd11e28ea9279fd7 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 9 Jan 2015 16:53:35 +0000 Subject: Modes; rework blocker for contentEditable. --- content_scripts/mode_insert.coffee | 2 ++ 1 file changed, 2 insertions(+) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index c0a61d31..5375bcdc 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -44,6 +44,7 @@ class InsertModeTrigger extends Mode # and unfortunately, the focus event happens *before* the change is made. Therefore, we need to # check again whether the active element is contentEditable. return @continueBubbling unless document.activeElement?.isContentEditable + console.log @count, @name, "fired (by keydown)" new InsertMode targetElement: document.activeElement @stopBubblingAndTrue @@ -52,6 +53,7 @@ class InsertModeTrigger extends Mode focus: (event) => triggerSuppressor.unlessSuppressed => return unless DomUtils.isFocusable event.target + console.log @count, @name, "fired (by focus)" new InsertMode targetElement: event.target -- cgit v1.2.3 From e2380ff9417834900649173750edf17a26a8b703 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 9 Jan 2015 18:12:58 +0000 Subject: Modes; block unmapped keys with contentEditable. --- content_scripts/mode_insert.coffee | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 5375bcdc..cbeea48f 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -15,8 +15,10 @@ class InsertMode extends Mode options = extend defaults, options options.exitOnBlur = options.targetElement || null super options + triggerSuppressor.suppress() exit: (event = null) -> + triggerSuppressor.unsuppress() super() if @options.blurOnExit element = event?.srcElement @@ -44,7 +46,6 @@ class InsertModeTrigger extends Mode # and unfortunately, the focus event happens *before* the change is made. Therefore, we need to # check again whether the active element is contentEditable. return @continueBubbling unless document.activeElement?.isContentEditable - console.log @count, @name, "fired (by keydown)" new InsertMode targetElement: document.activeElement @stopBubblingAndTrue @@ -53,7 +54,6 @@ class InsertModeTrigger extends Mode focus: (event) => triggerSuppressor.unlessSuppressed => return unless DomUtils.isFocusable event.target - console.log @count, @name, "fired (by focus)" new InsertMode targetElement: event.target @@ -89,6 +89,36 @@ class InsertModeBlocker extends Mode new @options.onClickMode targetElement: document.activeElement +# There's some unfortunate feature interaction with chrome's content editable handling. If the selection is +# content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard +# events. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. +# This mode sits near the bottom of the handler stack and suppresses keyboard events if: +# - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so +# on), and +# - 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 it shouldn't normally block other extensions or the page itself from +# handling keyboard events. +new class ContentEditableTrap extends Mode + constructor: -> + super + name: "content-editable-trap" + keydown: (event) => @handle => DomUtils.suppressPropagation event + keypress: (event) => @handle => @suppressEvent + keyup: (event) => @handle => @suppressEvent + + # True if the selection is content editable and a descendant of the active element. In this situation, + # chrome unilaterally focuses the element containing the anchor, dropping us into insert mode. + isContentEditableFocused: -> + element = document.getSelection()?.anchorNode?.parentElement + return element?.isContentEditable? and + document.activeElement? and + DomUtils.isDOMDescendant document.activeElement, element + + handle: (func) -> + if @isContentEditableFocused() then func() else @continueBubbling + root = exports ? window root.InsertMode = InsertMode root.InsertModeTrigger = InsertModeTrigger -- cgit v1.2.3 From d97e7786cb04dbbe5cae8e4b86e25437f66eb799 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 9 Jan 2015 22:08:37 +0000 Subject: Modes; fix focus return value for InsertModeTrigger. --- content_scripts/mode_insert.coffee | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index cbeea48f..9a2d5ce1 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -53,9 +53,10 @@ class InsertModeTrigger extends Mode @push focus: (event) => triggerSuppressor.unlessSuppressed => - return unless DomUtils.isFocusable event.target - new InsertMode - targetElement: event.target + @alwaysContinueBubbling => + if DomUtils.isFocusable event.target + new InsertMode + targetElement: event.target # We may already have focussed an input, so check. if document.activeElement and DomUtils.isEditable document.activeElement @@ -63,7 +64,7 @@ class InsertModeTrigger extends Mode targetElement: document.activeElement # Used by InsertModeBlocker to suppress InsertModeTrigger; see below. -triggerSuppressor = new Utils.Suppressor true +triggerSuppressor = new Utils.Suppressor true # Note: true == @continueBubbling # Suppresses InsertModeTrigger. This is used by various modes (usually by inheritance) to prevent # unintentionally dropping into insert mode on focusable elements. -- cgit v1.2.3 From ac90db47aa2671cd663cc6a9cdf783dc30a582e9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 00:00:11 +0000 Subject: Modes; more changes... - Better comments. - Strip unnecessary handlers for leaving post-find mode. - Simplify passKeys. - focusInput now re-bubbles its triggering keydown event. --- content_scripts/mode_insert.coffee | 53 +++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 26 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 9a2d5ce1..144b0be6 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -1,5 +1,5 @@ -# This mode is installed when insert mode is active. +# This mode is installed only when insert mode is active. class InsertMode extends Mode constructor: (options = {}) -> defaults = @@ -11,10 +11,10 @@ class InsertMode extends Mode keyup: (event) => @stopBubblingAndTrue exitOnEscape: true blurOnExit: true + targetElement: null - options = extend defaults, options - options.exitOnBlur = options.targetElement || null - super options + options.exitOnBlur ||= options.targetElement + super extend defaults, options triggerSuppressor.suppress() exit: (event = null) -> @@ -34,8 +34,8 @@ class InsertMode extends Mode # - On a keydown event in a contentEditable element. # - When a focusable element receives the focus. # -# The trigger can be suppressed via triggerSuppressor; see InsertModeBlocker, below. -# This mode is permanently installed fairly low down on the handler stack. +# The trigger can be suppressed via triggerSuppressor; see InsertModeBlocker, below. This mode is permanently +# installed (just above normal mode and passkeys mode) on the handler stack. class InsertModeTrigger extends Mode constructor: -> super @@ -44,7 +44,7 @@ class InsertModeTrigger extends Mode triggerSuppressor.unlessSuppressed => # Some sites (e.g. inbox.google.com) change the contentEditable attribute on the fly (see #1245); # and unfortunately, the focus event happens *before* the change is made. Therefore, we need to - # check again whether the active element is contentEditable. + # check (on every keydown) whether the active element is contentEditable. return @continueBubbling unless document.activeElement?.isContentEditable new InsertMode targetElement: document.activeElement @@ -58,7 +58,7 @@ class InsertModeTrigger extends Mode new InsertMode targetElement: event.target - # We may already have focussed an input, so check. + # We may have already focussed an input element, so check. if document.activeElement and DomUtils.isEditable document.activeElement new InsertMode targetElement: document.activeElement @@ -66,12 +66,13 @@ class InsertModeTrigger extends Mode # Used by InsertModeBlocker to suppress InsertModeTrigger; see below. triggerSuppressor = new Utils.Suppressor true # Note: true == @continueBubbling -# Suppresses InsertModeTrigger. This is used by various modes (usually by inheritance) to prevent +# Suppresses InsertModeTrigger. This is used by various modes (usually via inheritance) to prevent # unintentionally dropping into insert mode on focusable elements. class InsertModeBlocker extends Mode constructor: (options = {}) -> triggerSuppressor.suppress() options.name ||= "insert-blocker" + # See "click" handler below for an explanation of options.onClickMode. options.onClickMode ||= InsertMode super options @onExit -> triggerSuppressor.unsuppress() @@ -79,27 +80,29 @@ class InsertModeBlocker extends Mode @push "click": (event) => @alwaysContinueBubbling => - # The user knows best; so, if the user clicks on something, we get out of the way. + # The user knows best; so, if the user clicks on something, the insert-mode blocker gets out of the + # way. @exit event - # However, there's a corner case. If the active element is focusable, then we would have been in - # insert mode had we not been blocking the trigger. Now, clicking on the element will not generate - # a new focus event, so the insert-mode trigger will not fire. We have to handle this case - # specially. @options.onClickMode is the mode to use. + # However, there's a corner case. If the active element is focusable, then, had we not been + # blocking the trigger, we would already have been in insert mode. Now, a click on that element + # will not generate a new focus event, so the insert-mode trigger will not fire. We have to handle + # this case specially. @options.onClickMode specifies the mode to use (by default, insert mode). if document.activeElement and - event.target == document.activeElement and DomUtils.isEditable document.activeElement + event.target == document.activeElement and DomUtils.isEditable document.activeElement new @options.onClickMode targetElement: document.activeElement # There's some unfortunate feature interaction with chrome's content editable handling. If the selection is # content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard -# events. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. -# This mode sits near the bottom of the handler stack and suppresses keyboard events if: +# event. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. +# A single instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if: # - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so -# on), and +# on), # - 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 it shouldn't normally block other extensions or the page itself from +# circumstances outlined above. So, we shouldn't usually be blocking keyboard events for other extensions or +# the page itself. # handling keyboard events. new class ContentEditableTrap extends Mode constructor: -> @@ -109,17 +112,15 @@ new class ContentEditableTrap extends Mode keypress: (event) => @handle => @suppressEvent keyup: (event) => @handle => @suppressEvent - # True if the selection is content editable and a descendant of the active element. In this situation, - # chrome unilaterally focuses the element containing the anchor, dropping us into insert mode. + handle: (func) -> if @isContentEditableFocused() then func() else @continueBubbling + + # True if the selection is content editable and a descendant of the active element. isContentEditableFocused: -> element = document.getSelection()?.anchorNode?.parentElement - return element?.isContentEditable? and - document.activeElement? and + return element?.isContentEditable and + document.activeElement and DomUtils.isDOMDescendant document.activeElement, element - handle: (func) -> - if @isContentEditableFocused() then func() else @continueBubbling - root = exports ? window root.InsertMode = InsertMode root.InsertModeTrigger = InsertModeTrigger -- cgit v1.2.3 From fdcdd0113049042c94b2b56a6b716e2da58b860e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 08:25:02 +0000 Subject: Modes; instrument for debugging... - Set Mode.debug to true to see mode activation/deactivation on the console. - Use Mode.log() to see a list of currently-active modes. - Use handlerStack.debugOn() to enable debugging of the handler stack. --- content_scripts/mode_insert.coffee | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 144b0be6..7668d794 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -51,6 +51,7 @@ class InsertModeTrigger extends Mode @stopBubblingAndTrue @push + _name: "mode-#{@id}/activate-on-focus" focus: (event) => triggerSuppressor.unlessSuppressed => @alwaysContinueBubbling => @@ -78,6 +79,7 @@ class InsertModeBlocker extends Mode @onExit -> triggerSuppressor.unsuppress() @push + _name: "mode-#{@id}/bail-on-click" "click": (event) => @alwaysContinueBubbling => # The user knows best; so, if the user clicks on something, the insert-mode blocker gets out of the @@ -92,18 +94,18 @@ class InsertModeBlocker extends Mode new @options.onClickMode targetElement: document.activeElement -# There's some unfortunate feature interaction with chrome's content editable handling. If the selection is -# content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard -# event. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. -# A single instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if: -# - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so -# on), +# There's some unfortunate feature interaction with chrome's contentEditable handling. If the selection is +# contentEditable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard +# event. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. A single +# instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if: +# - they haven't been handled by any other mode (so not by normal mode, passkeys, insert, ...), # - 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. -# handling keyboard events. +# 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. new class ContentEditableTrap extends Mode constructor: -> super @@ -112,10 +114,10 @@ new class ContentEditableTrap extends Mode keypress: (event) => @handle => @suppressEvent keyup: (event) => @handle => @suppressEvent - handle: (func) -> if @isContentEditableFocused() then func() else @continueBubbling + handle: (func) -> if @wouldTriggerInsert() then func() else @continueBubbling # True if the selection is content editable and a descendant of the active element. - isContentEditableFocused: -> + wouldTriggerInsert: -> element = document.getSelection()?.anchorNode?.parentElement return element?.isContentEditable and document.activeElement and -- cgit v1.2.3 From 2199ad1bf9a7b063cc68a8e75f7a4a76ba125588 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 11:31:57 +0000 Subject: Modes; revert to master's handling of #1415. The behaviour in the situations described in #1415 require more thought and discussion. --- content_scripts/mode_insert.coffee | 29 ----------------------------- 1 file changed, 29 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 7668d794..1887adbc 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -94,35 +94,6 @@ class InsertModeBlocker extends Mode new @options.onClickMode targetElement: document.activeElement -# There's some unfortunate feature interaction with chrome's contentEditable handling. If the selection is -# contentEditable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard -# event. This has the unfortunate effect of dropping us unintentally into insert mode. See #1415. A single -# instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if: -# - they haven't been handled by any other mode (so not by normal mode, passkeys, insert, ...), -# - 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, although Option 3 would be a reasonable alternative. -new class ContentEditableTrap extends Mode - constructor: -> - super - name: "content-editable-trap" - keydown: (event) => @handle => DomUtils.suppressPropagation event - keypress: (event) => @handle => @suppressEvent - keyup: (event) => @handle => @suppressEvent - - handle: (func) -> if @wouldTriggerInsert() then func() else @continueBubbling - - # True if the selection is content editable and a descendant of the active element. - wouldTriggerInsert: -> - element = document.getSelection()?.anchorNode?.parentElement - return element?.isContentEditable and - document.activeElement and - DomUtils.isDOMDescendant document.activeElement, element - root = exports ? window root.InsertMode = InsertMode root.InsertModeTrigger = InsertModeTrigger -- cgit v1.2.3 From 35cb54fec7242fac5c68503a32ef9dd4fea5d9b6 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 12:17:22 +0000 Subject: Modes; minor changes. --- content_scripts/mode_insert.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 1887adbc..5720c901 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -1,5 +1,6 @@ -# This mode is installed only when insert mode is active. +# This mode is installed only when insert mode is active. It is a singleton, so a newly-activated instance +# displaces any active instance. class InsertMode extends Mode constructor: (options = {}) -> defaults = @@ -13,6 +14,7 @@ class InsertMode extends Mode blurOnExit: true targetElement: null + # If options.targetElement blurs, we exit. options.exitOnBlur ||= options.targetElement super extend defaults, options triggerSuppressor.suppress() -- cgit v1.2.3 From c554d1fd5b6d81506864516b6f86a14f8672bec5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 15:05:58 +0000 Subject: Modes; reinstate key blockers: - when the selection is contentEditable - in PostFindMode Restricted to printable characters. --- content_scripts/mode_insert.coffee | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'content_scripts/mode_insert.coffee') 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 -- cgit v1.2.3 From 704ae28629154a732e20e16d56b23af265d51b85 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 16:01:40 +0000 Subject: Modes; better printable detection, move to keyboard_utils. --- content_scripts/mode_insert.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index b907f22e..31bae8ec 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -121,7 +121,7 @@ new class ContentEditableTrap extends Mode element = document.getSelection()?.anchorNode?.parentElement return element?.isContentEditable and document.activeElement and - DomUtils. isPrintable event and + KeyboardUtils.isPrintable(event) and DomUtils.isDOMDescendant document.activeElement, element root = exports ? window -- cgit v1.2.3 From 80ad0bc3087a3bf00d61bdd6c9cf48e971e22480 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 10 Jan 2015 18:52:24 +0000 Subject: Modes; re-architect key suppression and passkeys. --- content_scripts/mode_insert.coffee | 28 ---------------------------- 1 file changed, 28 deletions(-) (limited to 'content_scripts/mode_insert.coffee') diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 31bae8ec..5720c901 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -96,34 +96,6 @@ 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 - KeyboardUtils.isPrintable(event) and - DomUtils.isDOMDescendant document.activeElement, element - root = exports ? window root.InsertMode = InsertMode root.InsertModeTrigger = InsertModeTrigger -- cgit v1.2.3