Closed Bug 1478410 Opened 3 years ago Closed 3 years ago

Split console does not close when codeMirror jsterm is enabled

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

Details

(Whiteboard: [boogaloo-mvp])

Attachments

(1 file)

This is probably due to CodeMirror intercepting the event and not bubbling it up like we are expecting it (the toolbox listen for `keypress` event on its document to detect <kbd>Esc</kbd> hit)
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [boogaloo-mvp]
Blocks: 1470922
so, this patch is not ideal. I'd rather still handle the Esc key in the extraKeys config like the other one.
This is doable only if we change how the toolbox listen for the Esc key (use keydown or keyup instead of keypress).
Sadly, this means that all the sub panels that need to handle the Escape key also need to drop keypress for keydown/keyup.
This sounds like something that we should do anyway (See Bug 968056) and would fit into DebTools.
I'll file a bug for changing the event in the toolbox, and another one, blocked on the first, to put back the Esc key handling in jsterm in extraKeys.

Brian, does that sound right to you ?
Comment on attachment 8994900 [details]
Bug 1478410 - Fix split console close in codeMirror jsterm; .

https://reviewboard.mozilla.org/r/259416/#review266694
Attachment #8994900 - Flags: review?(bgrinstead)
Comment on attachment 8994900 [details]
Bug 1478410 - Fix split console close in codeMirror jsterm; .

https://reviewboard.mozilla.org/r/259416/#review266706

::: commit-message-dd386:8
(Diff revision 2)
> +This patch removes the <kbd>Esc</kbd> handler from codeMirror
> +to put it on the jsterm-container. This prevent the interference
> +from codeMirror when we don't need to handle the event (i.e. it
> +should bubbles up to the toolbox where the split console state
> +is managed).
> +The webconsole_split test is  run with both old and

Nit: unnecessary double space

::: commit-message-dd386:8
(Diff revision 2)
> +This patch removes the <kbd>Esc</kbd> handler from codeMirror
> +to put it on the jsterm-container. This prevent the interference
> +from codeMirror when we don't need to handle the event (i.e. it
> +should bubbles up to the toolbox where the split console state
> +is managed).
> +The webconsole_split test is  run with both old and

Nit: unnecessary double space
Attachment #8994900 - Flags: review?(bgrinstead) → review+
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e56f7601927e
Fix split console close in codeMirror jsterm; r=bgrins.
https://hg.mozilla.org/mozilla-central/rev/e56f7601927e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.