Bug 1478410 - Fix split console close in codeMirror jsterm; r=bgrins. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 25 Jul 2018 18:26:27 +0200
changeset 823392 6b549379839dc824a38ee49ffd859092cfbecb13
parent 822981 4e6486b672b32aba075b704c6b1e41e8ccf7a135
push id117662
push userbmo:nchevobbe@mozilla.com
push dateFri, 27 Jul 2018 06:47:17 +0000
reviewersbgrins
bugs1478410
milestone63.0a1
Bug 1478410 - Fix split console close in codeMirror jsterm; r=bgrins. 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 new jsterm. MozReview-Commit-ID: BaLyj4wSdmv
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/test/mochitest/browser_webconsole_split.js
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -304,25 +304,16 @@ class JSTerm extends Component {
               ) {
                 return null;
               }
 
               this.clearCompletion();
               return "CodeMirror.Pass";
             },
 
-            "Esc": () => {
-              if (this.autocompletePopup.isOpen) {
-                this.clearCompletion();
-                return null;
-              }
-
-              return "CodeMirror.Pass";
-            },
-
             "PageUp": () => {
               if (this.autocompletePopup.isOpen) {
                 this.complete(this.COMPLETE_PAGEUP);
                 return null;
               }
 
               return "CodeMirror.Pass";
             },
@@ -348,25 +339,35 @@ class JSTerm extends Component {
             "End": () => {
               if (this.autocompletePopup.isOpen) {
                 this.autocompletePopup.selectedIndex =
                   this.autocompletePopup.itemCount - 1;
                 return null;
               }
 
               return "CodeMirror.Pass";
-            }
+            },
+
+            "Esc": false,
           }
         });
 
         this.editor.on("change", this._inputEventHandler);
         this.editor.appendToLocalElement(this.node);
         const cm = this.editor.codeMirror;
         cm.on("paste", (_, event) => this.props.onPaste(event));
         cm.on("drop", (_, event) => this.props.onPaste(event));
+
+        this.node.addEventListener("keydown", event => {
+          if (event.keyCode === KeyCodes.DOM_VK_ESCAPE && this.autocompletePopup.isOpen) {
+            this.clearCompletion();
+            event.preventDefault();
+            event.stopPropagation();
+          }
+        });
       }
     } else if (this.inputNode) {
       this.inputNode.addEventListener("keypress", this._keyPress);
       this.inputNode.addEventListener("input", this._inputEventHandler);
       this.inputNode.addEventListener("keyup", this._inputEventHandler);
       this.focus();
     }
 
--- a/devtools/client/webconsole/test/mochitest/browser_webconsole_split.js
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_split.js
@@ -7,21 +7,30 @@
 
 const TEST_URI = "data:text/html;charset=utf-8,Web Console test for splitting";
 const {Toolbox} = require("devtools/client/framework/toolbox");
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const L10N =
   new LocalizationHelper("devtools/client/locales/toolbox.properties");
 
 // Test is slow on Linux EC2 instances - Bug 962931
-requestLongerTimeout(2);
+requestLongerTimeout(4);
 
 add_task(async function() {
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
   let toolbox;
-
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
   await addTab(TEST_URI);
   await testConsoleLoadOnDifferentPanel();
   await testKeyboardShortcuts();
   await checkAllTools();
 
   info("Testing host types");
   checkHostType(Toolbox.HostType.BOTTOM);
   await checkToolboxUI();
@@ -244,9 +253,9 @@ add_task(async function() {
   }
 
   function checkHostType(hostType) {
     is(toolbox.hostType, hostType, "host type is " + hostType);
 
     const pref = Services.prefs.getCharPref("devtools.toolbox.host");
     is(pref, hostType, "host pref is " + hostType);
   }
-});
+}