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)
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)
(deleted),
patch
|
miker
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•10 years ago
|
||
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]
Assignee | ||
Comment 2•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
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
Assignee | ||
Comment 4•10 years ago
|
||
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 5•10 years ago
|
||
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+
Assignee | ||
Comment 6•10 years ago
|
||
(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.
No longer depends on: 1132557
Assignee | ||
Updated•10 years ago
|
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
Reporter | ||
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•