Closed Bug 999299 Opened 11 years ago Closed 10 years ago

Style Editor interferes with next / prev tool bindings

Categories

(DevTools :: Style Editor, defect, P2)

16 Branch
x86
macOS
defect

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: canuckistani, Assigned: bgrins)

References

(Blocks 1 open bug)

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(1 file)

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.
Old bug, might be shallow, super annoying when you run into it. Brian, when you get a chance can you assess the difficulty?
Flags: needinfo?(bgrinstead)
Priority: -- → P2
Whiteboard: [devedition-40]
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.
Flags: needinfo?(bgrinstead)
Whiteboard: [devedition-40] → [devedition-40][difficulty=easy]
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
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.
Depends on: 1132557
Attached patch styleeditor-brackets.patch (deleted) — Splinter Review
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
Attachment #8595027 - Flags: review?(mratcliffe)
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.
Attachment #8595027 - Flags: review?(mratcliffe) → review+
Blocks: 1156804
(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.
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: