999299 - Style Editor interferes with next / prev tool bindings

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect, P2)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

STR:

1. Open the style editor and focus the editor widget eg so that you could make changes
2. click on a different tool to the right of the style editor, eg Netmonitor
3. using the keybindings cmd + [, switch back to the style editor and keep going, as if you were trying to switch to the debugger

Expected: the toolbox should switch to the debugger

Actual: a '[' character is entered into the style editor

Note: from this point you can switch to the right again using CMD + ], so it seems to be the case of CMD+[ only that the use of the key combo isn't handled correctly.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Old bug, might be shallow, super annoying when you run into it. Brian, when you get a chance can you assess the difficulty?

Comment 2

[Brian Grinstead [:bgrins]]

The weird thing is that we are handling both key combos already: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/sourceeditor/editor.js#172.  I'm guessing CM is doing something special with '[' because it matches brackets in the closebrackets plugin.  We can add that keymap after the editor is initialized and it should work.

Comment 3

[Brian Grinstead [:bgrins]]

The best workaround is going to be upgrading codemirror 5 (Bug 1132557).  It seems to do a better job about not calling the closebrackets extension when there is a ctrl+cmd modifier with the key press.

I can add a quick workaround by not including '[]' as autoclose brackets in the Style Editor.  This will get rid of the bracket matching for attribute selectors, but it's probably worth that in order to fix the toolbox keybindings.

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: styleeditor-brackets.patch

Mike, what do you think of this workaround?  See comment 3 - this is the easiest way to make the style editor not interfere with the ctrl+[ keyboard shortcut.  Right now the closebrackets extension is taking over that keypress (even when ctrl/cmd is held down).  It's an easy fix, the only downside is that attribute selector brackets won't be auto matched.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc9867d6b7fd

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8595027 [details] [diff] [review]
styleeditor-brackets.patch

Review of attachment 8595027 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Brian Grinstead [:bgrins] from comment #4)
> Created attachment 8595027 [details] [diff] [review]
> styleeditor-brackets.patch
> 
> Mike, what do you think of this workaround?  See comment 3 - this is the
> easiest way to make the style editor not interfere with the ctrl+[ keyboard
> shortcut.  Right now the closebrackets extension is taking over that
> keypress (even when ctrl/cmd is held down).  It's an easy fix, the only
> downside is that attribute selector brackets won't be auto matched.
> 
> Try push:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc9867d6b7fd

This works but it is not ideal so we really should log another bug to use CM5 and undo this patch.

The oranges are from the telemetry test fix patch a.k.a. Teflon, which has been backed out since your try run.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #5)
> This works but it is not ideal so we really should log another bug to use
> CM5 and undo this patch.

Done, filed Bug 1156804 to undo this.  And Bug 1132557 is opened to do the CM upgrade.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5eb0386d2f13

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5eb0386d2f13