Bug 1260419 - part2: do not hide inplace editor autocomplete popup when scrolling;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 04 May 2016 16:39:29 +0200
changeset 363337 516307dbc3d6142e107a10e9c3fb44ee93acc551
parent 363336 5a4842974672bebc7427a546b4e4014201147465
child 520010 fbee76148950ed9fee7ff052f28dced688ed3acf
push id17174
push userjdescottes@mozilla.com
push dateWed, 04 May 2016 14:47:16 +0000
reviewerspbro
bugs1260419
milestone49.0a1
Bug 1260419 - part2: do not hide inplace editor autocomplete popup when scrolling;r=pbro MozReview-Commit-ID: W5N9tFCyOM
devtools/client/inspector/markup/test/helper_style_attr_test_runner.js
devtools/client/inspector/rules/test/browser_rules_completion-new-property_04.js
devtools/client/inspector/rules/test/browser_rules_completion-new-property_multiline.js
devtools/client/shared/autocomplete-popup.js
devtools/client/shared/inplace-editor.js
devtools/client/themes/common.css
--- a/devtools/client/inspector/markup/test/helper_style_attr_test_runner.js
+++ b/devtools/client/inspector/markup/test/helper_style_attr_test_runner.js
@@ -86,17 +86,16 @@ function enterData(data, editor, inspect
   return sendKey(key, editor, inspector);
 }
 
 function clickOnSuggestion(index, editor) {
   return new Promise(resolve => {
     info("Clicking on item " + index + " in the list");
     editor.once("after-suggest", () => executeSoon(resolve));
     editor.popup._list.childNodes[index].click();
-    editor.input.blur();
   });
 }
 
 function sendKey(key, editor, inspector) {
   return new Promise(resolve => {
     if (/(down|left|right|back_space|return)/ig.test(key)) {
       info("Adding event listener for down|left|right|back_space|return keys");
       editor.input.addEventListener("keypress", function onKeypress() {
--- a/devtools/client/inspector/rules/test/browser_rules_completion-new-property_04.js
+++ b/devtools/client/inspector/rules/test/browser_rules_completion-new-property_04.js
@@ -33,20 +33,20 @@ add_task(function* () {
 
   const itemIndex = 4;
   let bgcItem = editor.popup.getItemAtIndex(itemIndex);
   is(bgcItem.label, "background-color",
     "Check the expected completion element is background-color.");
   editor.popup.selectedIndex = itemIndex;
 
   info("Select the background-color suggestion with a mouse click.");
-  let onInputFocus = once(editor.input, "focus", true);
+  let onSuggest = editor.once("after-suggest");
   let node = editor.popup._list.childNodes[itemIndex];
   EventUtils.synthesizeMouseAtCenter(node, {}, view.styleWindow);
-  yield onInputFocus;
+  yield onSuggest;
   is(editor.input.value, "background-color", "Correct value is autocompleted");
 
   info("Press RETURN to move the focus to a property value editor.");
   let onModifications = waitForNEvents(view, "ruleview-changed", 2);
   EventUtils.synthesizeKey("VK_RETURN", {}, view.styleWindow);
   yield onModifications;
 
   // Getting the new value editor after focus
--- a/devtools/client/inspector/rules/test/browser_rules_completion-new-property_multiline.js
+++ b/devtools/client/inspector/rules/test/browser_rules_completion-new-property_multiline.js
@@ -89,20 +89,20 @@ add_task(function* () {
   ok(editor.popup.selectedIndex, 2, "Using DOWN cycles autocomplete values.");
   yield synthesizeKeyForAutocomplete("VK_UP", editor, view.styleWindow);
   is(editor.popup.selectedIndex, 1, "Using UP cycles autocomplete values.");
   item = editor.popup.getItemAtIndex(editor.popup.selectedIndex);
   is(item.label, "red", "Check autocomplete displays expected value.");
 
   info("Select the background-color suggestion with a mouse click.");
   let onRuleviewChanged = view.once("ruleview-changed");
-  let onInputFocus = once(editor.input, "focus", true);
+  let onSuggest = editor.once("after-suggest");
   let node = editor.popup._list.childNodes[editor.popup.selectedIndex];
   EventUtils.synthesizeMouseAtCenter(node, {}, view.styleWindow);
-  yield onInputFocus;
+  yield onSuggest;
   yield onRuleviewChanged;
 
   is(editor.input.value, EXPECTED_CSS_VALUE,
     "Input value correctly autocompleted");
 
   info("Press ESCAPE to leave the input.");
   onRuleviewChanged = view.once("ruleview-changed");
   EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
--- a/devtools/client/shared/autocomplete-popup.js
+++ b/devtools/client/shared/autocomplete-popup.js
@@ -31,19 +31,19 @@ const events = require("devtools/shared/
  */
 function AutocompletePopup(document, options = {}) {
   this._document = document;
 
   this.autoSelect = options.autoSelect || false;
   this.position = options.position || "after_start";
   this.direction = options.direction || "ltr";
 
-  this.onSelect = options.onSelect;
-  this.onClick = options.onClick;
-  this.onKeypress = options.onKeypress;
+  this.onSelectCallback = options.onSelect;
+  this.onClickCallback = options.onClick;
+  this.onKeypressCallback = options.onKeypress;
 
   let id = options.panelId || "devtools_autoCompletePopup";
   let theme = options.theme || "dark";
   // If theme is auto, use the devtools.theme pref
   if (theme == "auto") {
     theme = Services.prefs.getCharPref("devtools.theme");
     this.autoThemeEnabled = true;
     // Setup theme change listener.
@@ -88,43 +88,56 @@ function AutocompletePopup(document, opt
   this._list.setAttribute("flex", "1");
   this._list.setAttribute("seltype", "single");
 
   if (options.listBoxId) {
     this._list.setAttribute("id", options.listBoxId);
   }
   this._list.className = "devtools-autocomplete-listbox " + theme + "-theme";
 
-  if (this.onSelect) {
-    this._list.addEventListener("select", this.onSelect, false);
-  }
+  this.onSelect = this.onSelect.bind(this);
+  this.onClick = this.onClick.bind(this);
+  this.onKeypress = this.onKeypress.bind(this);
+  this._list.addEventListener("select", this.onSelect, false);
+  this._list.addEventListener("click", this.onClick, false);
+  this._list.addEventListener("keypress", this.onKeypress, false);
 
-  if (this.onClick) {
-    this._list.addEventListener("click", this.onClick, false);
-  }
-
-  if (this.onKeypress) {
-    this._list.addEventListener("keypress", this.onKeypress, false);
-  }
   this._itemIdCounter = 0;
 
   events.decorate(this);
 }
 exports.AutocompletePopup = AutocompletePopup;
 
 AutocompletePopup.prototype = {
   _document: null,
   _panel: null,
   _list: null,
   __scrollbarWidth: null,
 
   // Event handlers.
-  onSelect: null,
-  onClick: null,
-  onKeypress: null,
+  onSelect: function (e) {
+    this.emit("popup-select");
+    if (this.onSelectCallback) {
+      this.onSelectCallback(e);
+    }
+  },
+
+  onClick: function (e) {
+    this.emit("popup-click");
+    if (this.onClickCallback) {
+      this.onClickCallback(e);
+    }
+  },
+
+  onKeypress: function (e) {
+    this.emit("popup-keypress");
+    if (this.onKeypressCallback) {
+      this.onKeypressCallback(e);
+    }
+  },
 
   /**
    * Open the autocomplete popup panel.
    *
    * @param {nsIDOMNode} anchor
    *        Optional node to anchor the panel to.
    * @param {Number} xOffset
    *        Horizontal offset in pixels from the left of the node to the left
@@ -186,27 +199,19 @@ AutocompletePopup.prototype = {
    * same code. It is the responsability of the client code to perform DOM
    * cleanup.
    */
   destroy: function() {
     if (this.isOpen) {
       this.hidePopup();
     }
 
-    if (this.onSelect) {
-      this._list.removeEventListener("select", this.onSelect, false);
-    }
-
-    if (this.onClick) {
-      this._list.removeEventListener("click", this.onClick, false);
-    }
-
-    if (this.onKeypress) {
-      this._list.removeEventListener("keypress", this.onKeypress, false);
-    }
+    this._list.removeEventListener("select", this.onSelect, false);
+    this._list.removeEventListener("click", this.onClick, false);
+    this._list.removeEventListener("keypress", this.onKeypress, false);
 
     if (this.autoThemeEnabled) {
       gDevTools.off("pref-changed", this._handleThemeChange);
     }
 
     this._list.remove();
     this._panel.remove();
     this._document = null;
--- a/devtools/client/shared/inplace-editor.js
+++ b/devtools/client/shared/inplace-editor.js
@@ -28,16 +28,17 @@ const Services = require("Services");
 
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 const CONTENT_TYPES = {
   PLAIN_TEXT: 0,
   CSS_VALUE: 1,
   CSS_MIXED: 2,
   CSS_PROPERTY: 3,
 };
+const AUTOCOMPLETE_POPUP_CLASSNAME = "inplace-editor-autocomplete-popup";
 
 // The limit of 500 autocomplete suggestions should not be reached but is kept
 // for safety.
 const MAX_POPUP_ENTRIES = 500;
 
 const FOCUS_FORWARD = Ci.nsIFocusManager.MOVEFOCUS_FORWARD;
 const FOCUS_BACKWARD = Ci.nsIFocusManager.MOVEFOCUS_BACKWARD;
 
@@ -239,16 +240,17 @@ function InplaceEditor(options, event) {
                           ? false
                           : !!options.preserveTextStyles;
 
   this._onBlur = this._onBlur.bind(this);
   this._onWindowBlur = this._onWindowBlur.bind(this);
   this._onKeyPress = this._onKeyPress.bind(this);
   this._onInput = this._onInput.bind(this);
   this._onKeyup = this._onKeyup.bind(this);
+  this._onAutocompletePopupClick = this._onAutocompletePopupClick.bind(this);
 
   this._createInput();
 
   // Hide the provided element and add our editor.
   this.originalDisplay = this.elt.style.display;
   this.elt.style.display = "none";
   this.elt.parentNode.insertBefore(this.input, this.elt);
 
@@ -943,69 +945,80 @@ InplaceEditor.prototype = {
     }
 
     if (this._openPopupTimeout) {
       this.doc.defaultView.clearTimeout(this._openPopupTimeout);
     }
   },
 
   /**
-   * Handle loss of focus by calling done if it hasn't been called yet.
+   * Event handler called when the inplace-editor's input loses focus.
    */
   _onBlur: function(event) {
     if (event && this.popup && this.popup.isOpen &&
-        this.popup.selectedIndex >= 0) {
-      let label, preLabel;
-
-      if (this._selectedIndex === undefined) {
-        ({label, preLabel} =
-          this.popup.getItemAtIndex(this.popup.selectedIndex));
-      } else {
-        ({label, preLabel} = this.popup.getItemAtIndex(this._selectedIndex));
-      }
-
-      let input = this.input;
-      let pre = "";
+      this.popup.selectedIndex >= 0) {
+      this._acceptPopupSuggestion();
+    } else {
+      this._apply();
+      this._clear();
+    }
+  },
 
-      // CSS_MIXED needs special treatment here to make it so that
-      // multiple presses of tab will cycle through completions, but
-      // without selecting the completed text.  However, this same
-      // special treatment will do the wrong thing for other editing
-      // styles.
-      if (input.selectionStart < input.selectionEnd ||
-          this.contentType !== CONTENT_TYPES.CSS_MIXED) {
-        pre = input.value.slice(0, input.selectionStart);
-      } else {
-        pre = input.value.slice(0, input.selectionStart - label.length +
-                                preLabel.length);
-      }
-      let post = input.value.slice(input.selectionEnd, input.value.length);
-      let item = this.popup.selectedItem;
-      this._selectedIndex = this.popup.selectedIndex;
-      let toComplete = item.label.slice(item.preLabel.length);
-      input.value = pre + toComplete + post;
-      input.setSelectionRange(pre.length + toComplete.length,
-                              pre.length + toComplete.length);
-      this._updateSize();
-      // Wait for the popup to hide and then focus input async otherwise it does
-      // not work.
-      let onPopupHidden = () => {
-        this.popup._panel.removeEventListener("popuphidden", onPopupHidden);
-        this.doc.defaultView.setTimeout(()=> {
-          input.focus();
-          this.emit("after-suggest");
-        }, 0);
-      };
-      this.popup._panel.addEventListener("popuphidden", onPopupHidden);
-      this.popup.hidePopup();
-      return;
+  /**
+   * Event handler called by the autocomplete popup when receiving a click
+   * event.
+   */
+  _onAutocompletePopupClick: function() {
+    this._acceptPopupSuggestion();
+  },
+
+  _acceptPopupSuggestion: function() {
+    let label, preLabel;
+
+    if (this._selectedIndex === undefined) {
+      ({label, preLabel} =
+        this.popup.getItemAtIndex(this.popup.selectedIndex));
+    } else {
+      ({label, preLabel} = this.popup.getItemAtIndex(this._selectedIndex));
     }
 
-    this._apply();
-    this._clear();
+    let input = this.input;
+    let pre = "";
+
+    // CSS_MIXED needs special treatment here to make it so that
+    // multiple presses of tab will cycle through completions, but
+    // without selecting the completed text.  However, this same
+    // special treatment will do the wrong thing for other editing
+    // styles.
+    if (input.selectionStart < input.selectionEnd ||
+        this.contentType !== CONTENT_TYPES.CSS_MIXED) {
+      pre = input.value.slice(0, input.selectionStart);
+    } else {
+      pre = input.value.slice(0, input.selectionStart - label.length +
+                              preLabel.length);
+    }
+    let post = input.value.slice(input.selectionEnd, input.value.length);
+    let item = this.popup.selectedItem;
+    this._selectedIndex = this.popup.selectedIndex;
+    let toComplete = item.label.slice(item.preLabel.length);
+    input.value = pre + toComplete + post;
+    input.setSelectionRange(pre.length + toComplete.length,
+                            pre.length + toComplete.length);
+    this._updateSize();
+    // Wait for the popup to hide and then focus input async otherwise it does
+    // not work.
+    let onPopupHidden = () => {
+      this.popup._panel.removeEventListener("popuphidden", onPopupHidden);
+      this.doc.defaultView.setTimeout(()=> {
+        input.focus();
+        this.emit("after-suggest");
+      }, 0);
+    };
+    this.popup._panel.addEventListener("popuphidden", onPopupHidden);
+    this._hideAutocompletePopup();
   },
 
   /**
    * Handle the input field's keypress event.
    */
   _onKeyPress: function(event) {
     let prevent = false;
 
@@ -1037,17 +1050,17 @@ InplaceEditor.prototype = {
       prevent = true;
       cycling = true;
       this._cycleCSSSuggestion(isKeyIn(key, "UP", "PAGE_UP"));
       this._doValidation();
     }
 
     if (isKeyIn(key, "BACK_SPACE", "DELETE", "LEFT", "RIGHT")) {
       if (isPopupOpen) {
-        this.popup.hidePopup();
+        this._hideAutocompletePopup();
       }
     } else if (!cycling && !multilineNavigation &&
       !event.metaKey && !event.altKey && !event.ctrlKey) {
       this._maybeSuggestCompletion(true);
     }
 
     if (this.multiline && event.shiftKey && isKeyIn(key, "RETURN")) {
       prevent = false;
@@ -1088,17 +1101,17 @@ InplaceEditor.prototype = {
           return;
         }
       }
 
       this._apply(event, direction);
 
       // Close the popup if open
       if (this.popup && this.popup.isOpen) {
-        this.popup.hidePopup();
+        this._hideAutocompletePopup();
       }
 
       if (direction !== null && focusManager.focusedElement === input) {
         // If the focused element wasn't changed by the done callback,
         // move the focus as requested.
         let next = moveFocus(this.doc.defaultView, direction);
 
         // If the next node to be focused has been tagged as an editable
@@ -1112,17 +1125,17 @@ InplaceEditor.prototype = {
 
       this._clear();
     } else if (isKeyIn(key, "ESCAPE")) {
       // Cancel and blur ourselves.
       // Now we don't want to suggest anything as we are moving out.
       this._preventSuggestions = true;
       // Close the popup if open
       if (this.popup && this.popup.isOpen) {
-        this.popup.hidePopup();
+        this._hideAutocompletePopup();
       }
       prevent = true;
       this.cancelled = true;
       this._apply();
       this._clear();
       event.stopPropagation();
     } else if (isKeyIn(key, "SPACE")) {
       // No need for leading spaces here.  This is particularly
@@ -1132,16 +1145,41 @@ InplaceEditor.prototype = {
     }
 
     if (prevent) {
       event.preventDefault();
     }
   },
 
   /**
+   * Open the autocomplete popup, adding a custom click handler and classname.
+   *
+   * @param {Number} offset
+   *        X-offset relative to the input starting edge.
+   * @param {Number} selectedIndex
+   *        The index of the item that should be selected. Use -1 to have no
+   *        item selected.
+   */
+  _openAutocompletePopup: function(offset, selectedIndex) {
+    this.popup._panel.classList.add(AUTOCOMPLETE_POPUP_CLASSNAME);
+    this.popup.on("popup-click", this._onAutocompletePopupClick);
+    this.popup.openPopup(this.input, offset, 0, selectedIndex);
+  },
+
+  /**
+   * Remove the custom classname and click handler and close the autocomplete
+   * popup.
+   */
+  _hideAutocompletePopup: function() {
+    this.popup._panel.classList.remove(AUTOCOMPLETE_POPUP_CLASSNAME);
+    this.popup.off("popup-click", this._onAutocompletePopupClick);
+    this.popup.hidePopup();
+  },
+
+  /**
    * Get the increment/decrement step to use for the provided key event.
    */
   _getIncrement: function(event) {
     const largeIncrement = 100;
     const mediumIncrement = 10;
     const smallIncrement = 0.1;
 
     let increment = 0;
@@ -1378,19 +1416,19 @@ InplaceEditor.prototype = {
         let offset = indent * this.inputCharDimensions.width;
         offset = this._isSingleLine() ? offset : 0;
 
         // Select the most relevantItem if autoInsert is allowed
         let selectedIndex = autoInsert ? index : -1;
 
         // Open the suggestions popup.
         this.popup.setItems(finalList);
-        this.popup.openPopup(this.input, offset, 0, selectedIndex);
+        this._openAutocompletePopup(offset, selectedIndex);
       } else {
-        this.popup.hidePopup();
+        this._hideAutocompletePopup();
       }
       // This emit is mainly for the purpose of making the test flow simpler.
       this.emit("after-suggest");
       this._doValidation();
     }, 0);
   },
 
   /**
--- a/devtools/client/themes/common.css
+++ b/devtools/client/themes/common.css
@@ -56,16 +56,22 @@
   border-width: 0px !important;
   margin: 0;
 }
 
 .devtools-autocomplete-listbox > scrollbox {
   padding: 2px;
 }
 
+.inplace-editor-autocomplete-popup .devtools-autocomplete-listbox {
+  /* Inplace editor closes the autocomplete popup on blur, the autocomplete
+  popup should not steal the focus here.*/
+  -moz-user-focus: ignore;
+}
+
 .devtools-autocomplete-listbox > richlistitem,
 .devtools-autocomplete-listbox > richlistitem[selected] {
   width: 100%;
   background-color: transparent;
   border-radius: 4px;
 }
 
 .devtools-autocomplete-listbox.dark-theme > richlistitem[selected],