Bug 919232 - arrow keys not working in form autofill popup r=mbrubeck
authorRodrigo Silveira <rsilveira@mozilla.com>
Wed, 16 Oct 2013 14:41:42 -0700
changeset 153153 602cf7b9f336f67eec3370feb1567747a220e118
parent 153152 8658f9b994541dc1e56008fc75987ef202f0d826
child 153154 c672c4cd6c7fa74591c742867c40295ee02253ec
push id3299
push userrsilveira@mozilla.com
push dateFri, 01 Nov 2013 23:00:19 +0000
treeherderfx-team@602cf7b9f336 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmbrubeck
bugs919232
milestone28.0a1
Bug 919232 - arrow keys not working in form autofill popup r=mbrubeck
browser/metro/base/content/contenthandlers/Content.js
browser/metro/base/content/contenthandlers/FormHelper.js
browser/metro/base/content/helperui/MenuUI.js
browser/metro/base/content/helperui/SelectHelperUI.js
browser/metro/base/tests/mochitest/browser_form_auto_complete.js
browser/metro/theme/platform.css
--- a/browser/metro/base/content/contenthandlers/Content.js
+++ b/browser/metro/base/content/contenthandlers/Content.js
@@ -164,28 +164,25 @@ let Content = {
         sendAsyncMessage("Browser:MozApplicationManifest", {
           location: doc.documentURIObject.spec,
           manifest: doc.documentElement.getAttribute("manifest"),
           charset: doc.characterSet
         });
         break;
       }
 
-      case "keydown":
-        if (aEvent.keyCode == aEvent.DOM_VK_ESCAPE)
-          this.formAssistant.close();
-        break;
-
       case "keyup":
         // If after a key is pressed we still have no input, then close
-        // the autocomplete.  Perhaps the user used backspace or delete.
-        if (!aEvent.target.value)
+        // the autocomplete. Perhaps the user used backspace or delete.
+        // Allow down arrow to trigger autofill popup on empty input.
+        if ((!aEvent.target.value && aEvent.keyCode != aEvent.DOM_VK_DOWN)
+          || aEvent.keyCode == aEvent.DOM_VK_ESCAPE)
           this.formAssistant.close();
         else
-          this.formAssistant.open(aEvent.target);
+          this.formAssistant.open(aEvent.target, aEvent);
         break;
 
       case "click":
         // Workaround for bug 925457: we sometimes don't recognize the
         // correct tap target or are unable to identify if it's editable.
         // Instead always save tap co-ordinates for the keyboard to look for
         // when it is up.
         SelectionHandler.onClickCoords(aEvent.clientX, aEvent.clientY);
--- a/browser/metro/base/content/contenthandlers/FormHelper.js
+++ b/browser/metro/base/content/contenthandlers/FormHelper.js
@@ -30,24 +30,22 @@ function FormAssistant() {
   addMessageListener("FormAssist:ChoiceChange", this);
   addMessageListener("FormAssist:AutoComplete", this);
   addMessageListener("FormAssist:Update", this);
 
   /* Listen text events in order to update the autocomplete suggestions as soon
    * a key is entered on device
    */
   addEventListener("text", this, false);
-  addEventListener("keypress", this, true);
-  addEventListener("keyup", this, false);
   addEventListener("focus", this, true);
   addEventListener("blur", this, true);
   addEventListener("pageshow", this, false);
   addEventListener("pagehide", this, false);
   addEventListener("submit", this, false);
-};
+}
 
 FormAssistant.prototype = {
   _els: Cc["@mozilla.org/eventlistenerservice;1"].getService(Ci.nsIEventListenerService),
   _open: false,
   _focusSync: false,
   _debugEvents: false,
   _selectWrapper: null,
   _currentElement: null,
@@ -98,18 +96,18 @@ FormAssistant.prototype = {
 
     // Don't show the formhelper popup for multi-select boxes, except for touch.
     if (aElement instanceof HTMLSelectElement && aEvent) {
       if ((aElement.multiple || aElement.size > 1) &&
           aEvent.mozInputSource != Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH) {
         return false;
       }
       // Don't fire mouse events on selects; see bug 685197.
-      aEvent.preventDefault()
-      aEvent.stopPropagation()
+      aEvent.preventDefault();
+      aEvent.stopPropagation();
     }
 
     // The form assistant will close if a click happen:
     // * outside of the scope of the form helper
     // * hover a button of type=[image|submit]
     // * hover a disabled element
     if (!this._isValidElement(aElement)) {
       let passiveButtons = { button: true, checkbox: true, file: true, radio: true, reset: true };
@@ -125,16 +123,21 @@ FormAssistant.prototype = {
     }
 
     // We only work with choice lists or elements with autocomplete suggestions
     if (!this._isSelectElement(aElement) &&
         !this._isAutocomplete(aElement)) {
       return this.close();
     }
 
+    // Don't re-open when navigating to avoid repopulating list when changing selection.
+    if (this._isAutocomplete(aElement) && this._open && this._isNavigationKey(aEvent)) {
+      return false;
+    }
+
     // Enable the assistant
     this.currentElement = aElement;
     return this._open = true;
   },
 
   close: function close() {
     if (this._open) {
       this._currentElement = null;
@@ -162,16 +165,17 @@ FormAssistant.prototype = {
         this._selectWrapper = getWrapperForElement(currentElement);
         this._selectWrapper.select(json.index, json.selected);
         break;
       }
 
       case "FormAssist:ChoiceChange": {
         // ChoiceChange could happened once we have move to another element or
         // to nothing, so we should keep the used wrapper in mind.
+        this._selectWrapper = getWrapperForElement(currentElement);
         this._selectWrapper.fireOnChange();
 
         // New elements can be shown when a select is updated so we need to
         // reconstruct the inner elements array and to take care of possible
         // focus change, this is why we use "self.currentElement" instead of 
         // using directly "currentElement".
         this._executeDelayed(function(self) {
           let currentElement = self.currentElement;
@@ -281,31 +285,33 @@ FormAssistant.prototype = {
         }, 0, this);
         break;
 
       case "text":
         if (this._isAutocomplete(aEvent.target)) {
           this._sendJsonMsgWrapper("FormAssist:AutoComplete");
         }
         break;
-
-      case "keyup":
-        // There is no need to handle keys if there is not element currently
-        // used by the form assistant
-        if (!currentElement)
-          return;
+    }
+  },
 
-        if (this._isAutocomplete(aEvent.target)) {
-          this._sendJsonMsgWrapper("FormAssist:AutoComplete");
-        }
+  _isNavigationKey: function (aEvent) {
+    // Ignore navigation keys
+    if (aEvent.keyCode) {
+      let navigationKeys = [
+        aEvent.DOM_VK_DOWN,
+        aEvent.DOM_VK_UP,
+        aEvent.DOM_VK_LEFT,
+        aEvent.DOM_VK_RIGHT,
+        aEvent.DOM_VK_PAGE_UP,
+        aEvent.DOM_VK_PAGE_DOWN];
 
-        let caretRect = this._getCaretRect();
-        if (!caretRect.isEmpty())
-          sendAsyncMessage("FormAssist:Update", { caretRect: caretRect });
+      return navigationKeys.indexOf(aEvent.keyCode) != -1;
     }
+    return false;
   },
 
   _executeDelayed: function formHelperExecuteSoon(aCallback) {
     let self = this;
     let timer = new Util.Timeout(function() {
       aCallback(self);
     });
     timer.once(0);
@@ -635,17 +641,17 @@ function getListForElement(aElement) {
         disabled: child.disabled,
         selected: child.selected,
         optionIndex: optionIndex++
       });
     }
   }
 
   return result;
-};
+}
 
 
 function SelectWrapper(aControl) {
   this._control = aControl;
 }
 
 SelectWrapper.prototype = {
   getSelectedIndex: function() {
--- a/browser/metro/base/content/helperui/MenuUI.js
+++ b/browser/metro/base/content/helperui/MenuUI.js
@@ -44,37 +44,75 @@ var AutofillMenuUI = {
       yPos: this._anchorRect.y + this._anchorRect.height,
       maxWidth: this._anchorRect.width,
       maxHeight: 350,
       source: Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH
     };
   },
 
   show: function show(aAnchorRect, aSuggestionsList) {
+    this.commands.addEventListener("select", this, true);
+    window.addEventListener("keypress", this, true);
+
     this._anchorRect = aAnchorRect;
     this._emptyCommands();
     for (let idx = 0; idx < aSuggestionsList.length; idx++) {
       let item = document.createElement("richlistitem");
       let label = document.createElement("label");
       label.setAttribute("value", aSuggestionsList[idx].label);
+      item.setAttribute("value", aSuggestionsList[idx].value);
       item.setAttribute("data", aSuggestionsList[idx].value);
       item.appendChild(label);
       this.commands.appendChild(item);
     }
-
     this._menuPopup.show(this._positionOptions());
   },
 
   selectByIndex: function mn_selectByIndex(aIndex) {
     this._menuPopup.hide();
     FormHelperUI.doAutoComplete(this.commands.childNodes[aIndex].getAttribute("data"));
   },
 
   hide: function hide () {
+    window.removeEventListener("keypress", this, true);
+    this.commands.removeEventListener("select", this, true);
+
     this._menuPopup.hide();
+  },
+
+  handleEvent: function (aEvent) {
+    switch (aEvent.type) {
+      case "keypress":
+        switch (aEvent.keyCode) {
+          case aEvent.DOM_VK_ESCAPE:
+            this.hide();
+            break;
+
+          case aEvent.DOM_VK_DOWN:
+            this.commands.moveByOffset(1, true, false);
+            break;
+
+          case aEvent.DOM_VK_UP:
+            this.commands.moveByOffset(-1, true, false);
+            break;
+
+          case aEvent.DOM_VK_PAGE_DOWN:
+            this.commands.moveByOffset(this.commands.scrollOnePage(1), true, false);
+            break;
+
+          case aEvent.DOM_VK_PAGE_UP:
+            this.commands.moveByOffset(this.commands.scrollOnePage(-1), true, false);
+            break;
+        }
+        break;
+
+      case "select":
+        FormHelperUI.doAutoComplete(this.commands.value);
+        break;
+    }
   }
 };
 
 var ContextMenuUI = {
   _popupState: null,
   __menuPopup: null,
   _defaultPositionOptions: {
     bottomAligned: true,
@@ -341,19 +379,17 @@ function MenuPopup(aPanel, aPopup) {
 
   window.addEventListener('MozAppbarShowing', this, false);
 }
 MenuPopup.prototype = {
   get visible() { return !this._panel.hidden; },
   get commands() { return this._popup.childNodes[0]; },
 
   show: function (aPositionOptions) {
-    if (this.visible) {
-      this._animateHide().then(() => this._animateShow(aPositionOptions));
-    } else {
+    if (!this.visible) {
       this._animateShow(aPositionOptions);
     }
   },
 
   hide: function () {
     if (this.visible) {
       this._animateHide();
     }
@@ -423,16 +459,17 @@ MenuPopup.prototype = {
 
   _animateShow: function (aPositionOptions) {
     let deferred = Promise.defer();
 
     window.addEventListener("keypress", this, true);
     window.addEventListener("mousedown", this, true);
     window.addEventListener("touchstart", this, true);
     window.addEventListener("scroll", this, true);
+    window.addEventListener("blur", this, true);
     Elements.stack.addEventListener("PopupChanged", this, false);
 
     this._panel.hidden = false;
     let popupFrom = !aPositionOptions.bottomAligned ? "above" : "below";
     this._panel.setAttribute("showingfrom", popupFrom);
 
     // This triggers a reflow, which sets transitionability.
     // All animation/transition setup must happen before here.
@@ -453,16 +490,17 @@ MenuPopup.prototype = {
 
   _animateHide: function () {
     let deferred = Promise.defer();
 
     window.removeEventListener("keypress", this, true);
     window.removeEventListener("mousedown", this, true);
     window.removeEventListener("touchstart", this, true);
     window.removeEventListener("scroll", this, true);
+    window.removeEventListener("blur", this, true);
     Elements.stack.removeEventListener("PopupChanged", this, false);
 
     let self = this;
     this._panel.addEventListener("transitionend", function popuphidden() {
       self._panel.removeEventListener("transitionend", popuphidden);
       self._panel.removeAttribute("hiding");
       self._panel.hidden = true;
       self._popup.style.maxWidth = "none";
@@ -489,16 +527,17 @@ MenuPopup.prototype = {
         if (!this._wantTypeBehind) {
           // Hide the context menu so you can't type behind it.
           aEvent.stopPropagation();
           aEvent.preventDefault();
           if (aEvent.keyCode != aEvent.DOM_VK_ESCAPE)
             this.hide();
         }
         break;
+      case "blur":
       case "mousedown":
       case "touchstart":
       case "scroll":
         if (!this._popup.contains(aEvent.target)) {
           aEvent.stopPropagation();
           this.hide();
         }
         break;
--- a/browser/metro/base/content/helperui/SelectHelperUI.js
+++ b/browser/metro/base/content/helperui/SelectHelperUI.js
@@ -38,25 +38,27 @@ var SelectHelperUI = {
 
     let firstSelected = null;
 
     // Using a fragment prevent us to hang on huge list
     let fragment = document.createDocumentFragment();
     let choices = aList.choices;
     for (let i = 0; i < choices.length; i++) {
       let choice = choices[i];
-      let item = document.createElement("listitem");
+      let item = document.createElement("richlistitem");
+      let label = document.createElement("label");
 
-      item.setAttribute("class", "option-command listitem-iconic action-button");
+      item.setAttribute("class", "option-command listitem-iconic");
       item.setAttribute("flex", "1");
       item.setAttribute("crop", "center");
-      item.setAttribute("label", choice.text);
+      label.setAttribute("value", choice.text);
+      item.appendChild(label);
 
-      choice.selected ? item.classList.add("selected")
-                      : item.classList.remove("selected");
+      choice.selected ? item.setAttribute("selected", "true")
+                      : item.removeAttribute("selected");
 
       choice.disabled ? item.setAttribute("disabled", "true")
                       : item.removeAttribute("disabled");
       fragment.appendChild(item);
 
       if (choice.group) {
         item.classList.add("optgroup");
         continue;
--- a/browser/metro/base/tests/mochitest/browser_form_auto_complete.js
+++ b/browser/metro/base/tests/mochitest/browser_form_auto_complete.js
@@ -5,22 +5,22 @@
 
 "use strict";
 
 function clearFormHistory() {
   FormHistory.update({ op : "remove" });
 }
 
 function test() {
-  clearFormHistory();
   runTests();
   clearFormHistory();
 }
 
 function setUp() {
+  clearFormHistory();
   PanelUI.hide();
   yield hideContextUI();
 }
 
 function tearDown() {
   PanelUI.hide();
 }
 
@@ -79,9 +79,44 @@ gTests.push({
     // first click is ignored.
     shownPromise = waitForEvent(document, "popupshown");
     EventUtils.synthesizeMouseAtCenter(input, {}, Browser.selectedTab.browser.contentWindow);
     EventUtils.synthesizeMouseAtCenter(input, {}, Browser.selectedTab.browser.contentWindow);
     yield shownPromise;
 
     checkAutofillMenuItemContents(["hellothere", "one", "two", "three", "four", "five"]);
   }
-});
\ No newline at end of file
+});
+
+gTests.push({
+  desc: "Test autocomplete selection with arrow key.",
+  setUp: setUp,
+  tearDown: tearDown,
+  run: function () {
+
+    let newTab = yield addTab(chromeRoot + "browser_form_auto_complete.html");
+    yield waitForCondition(function () {
+      return !Browser.selectedTab.isLoading();
+    });
+
+    let tabDocument = newTab.browser.contentWindow.document;
+    let input = tabDocument.getElementById("textedit1");
+    input.focus();
+
+    let shownPromise = waitForEvent(document, "popupshown");
+    EventUtils.synthesizeKey("o", {}, window);
+    yield shownPromise;
+
+    EventUtils.synthesizeKey("VK_DOWN", {}, window);
+
+    yield waitForCondition(() => input.value == "one");
+
+    is(input.value, "one", "Input updated correctly");
+
+    EventUtils.synthesizeKey("VK_DOWN", {}, window);
+
+    yield waitForCondition(() => input.value == "two");
+
+    is(input.value, "two", "Input updated correctly");
+
+    Browser.closeTab(newTab, { forceClose: true });
+  }
+});
--- a/browser/metro/theme/platform.css
+++ b/browser/metro/theme/platform.css
@@ -192,26 +192,16 @@ menulist {
   width: 100%;
   min-height: @touch_button_small@;
   min-width: @touch_action_minwidth@; /* keep the button from being too narrow */
   border: 0 none;
   -moz-box-align: center;
   font-weight: 600;
 }
 
-.menu-popup richlistitem:not([disabled]):hover {
-  background-color: #ccc;
-  color: black;
-}
-
-.menu-popup richlistitem:not([disabled]):active {
-  background-color: black;
-  color: white;
-}
-
 .menu-popup > richlistbox > richlistitem {
   padding-right: 50px;
 }
 
 /* Additional styles applied to popups for form <select> elements. */
 
 #select-container {
   padding: 0;
@@ -246,26 +236,16 @@ menulist {
 }
 
 .option-command.optgroup {
   font-weight: bold;
   font-style: italic;
   pointer-events: none;
 }
 
-.option-command:not([disabled]):hover {
-  background-color: #f0f0f0;
-  color: black;
-}
-
-.option-command:not([disabled]):active {
-  background-color: #d3d3d3;
-  color: black;
-}
-
 .select-popup > richlistbox > scrollbox {
   width: 100%;
   overflow-x: hidden !important;
 }
 
 /* Toolbar Button --------------------------------------------------------- */
 
 toolbarbutton {
@@ -399,23 +379,24 @@ richlistitem description.normal {
   color: gray;
 }
 
 richlistitem label.normal-bold,
 richlistitem description.normal-bold {
   font-weight: bold;
 }
 
-richlistitem[selected] {
-  color: black;
-  background-color: white;
+richlistitem[selected],
+richlistitem:not([disabled]):not([selected]):active {
+  background-color: @metro_orange@;
+  color: #fff;
 }
 
-richlistitem:hover:active:not([selected]) {
-  background-color: #8db8d8;
+richlistitem:not([disabled]):not([selected]):hover {
+  background-color: #ccc;
 }
 
 richlistitem.section-header,
 richlistitem[selected].section-header {
   font-weight: bold;
   color: #000;
   background-color: lightgray;
 }