Bug 1300996 - Part 1: Set selectedIndex of autocomplete popup when mousemove over profile item. r=adw
☠☠ backed out by f6e748977f2e ☠ ☠
authorRay Lin <ralin@mozilla.com>
Tue, 25 Apr 2017 16:48:52 +0800
changeset 411072 baea1f6031e8c57e5f899223e992a56bdd21c30a
parent 411066 f46c3d67b5eb210dd769c68b9e3d34841a756824
child 411073 97543ecd15b617945f2cc8ebb41568f8c95d2ec4
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1300996
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1300996 - Part 1: Set selectedIndex of autocomplete popup when mousemove over profile item. r=adw Currently mouseover autocomplete item would not change the selectedIndex of popup but show grey highlight. For form autofill, we don't want two indicators to represent the selection of keyboard and mouse separately that might confuse user which profile preview is exactly being shown. I added a new <field> in formautofill profile item binding that helps autocomplete to determine whether to change selectedIndex when mouseover. MozReview-Commit-ID: LmW8g08V9mf
browser/extensions/formautofill/content/formautofill.xml
toolkit/components/satchel/test/browser/browser.ini
toolkit/components/satchel/test/browser/browser_popup_mouseover.js
toolkit/content/widgets/autocomplete.xml
toolkit/content/widgets/richlistbox.xml
--- a/browser/extensions/formautofill/content/formautofill.xml
+++ b/browser/extensions/formautofill/content/formautofill.xml
@@ -22,16 +22,22 @@
         </div>
         <div class="profile-comment-col profile-item-col">
           <span anonid="profile-comment" class="profile-comment"></span>
         </div>
       </div>
     </xbl:content>
 
     <implementation implements="nsIDOMXULSelectControlItemElement">
+      <!-- For form autofill, we want to unify the selection no matter by
+      keyboard navigation or mouseover in order not to confuse user which
+      profile preview is being shown. This field is set to true to indicate
+      that selectedIndex of popup should be changed while mouseover item -->
+      <field name="selectedByMouseOver">true</field>
+
       <constructor>
         <![CDATA[
           this._itemBox = document.getAnonymousElementByAttribute(
             this, "anonid", "profile-item-box"
           );
           this._label = document.getAnonymousElementByAttribute(
             this, "anonid", "profile-label"
           );
--- a/toolkit/components/satchel/test/browser/browser.ini
+++ b/toolkit/components/satchel/test/browser/browser.ini
@@ -1,5 +1,6 @@
 [DEFAULT]
 support-files =
   !/toolkit/components/satchel/test/subtst_privbrowsing.html
 
+[browser_popup_mouseover.js]
 [browser_privbrowsing_perwindowpb.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/satchel/test/browser/browser_popup_mouseover.js
@@ -0,0 +1,54 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const {FormHistory} = Cu.import("resource://gre/modules/FormHistory.jsm", {});
+
+
+add_task(async function test() {
+  const url = `data:text/html,<input type="text" name="field1">`;
+  await BrowserTestUtils.withNewTab({gBrowser, url}, async function(browser) {
+    const {autoCompletePopup, autoCompletePopup: {richlistbox: itemsBox}} = browser;
+    const mockHistory = [
+      {op: "add", fieldname: "field1", value: "value1"},
+      {op: "add", fieldname: "field1", value: "value2"},
+      {op: "add", fieldname: "field1", value: "value3"},
+      {op: "add", fieldname: "field1", value: "value4"},
+    ];
+
+    await new Promise(resolve => FormHistory.update([{op: "remove"}, ...mockHistory], {handleCompletion: resolve}));
+    await ContentTask.spawn(browser, {}, async function() {
+      const input = content.document.querySelector("input");
+
+      input.focus();
+    });
+
+    // show popup
+    await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
+    await BrowserTestUtils.waitForCondition(() => {
+      return autoCompletePopup.popupOpen;
+    });
+    const listItemElems = itemsBox.querySelectorAll(".autocomplete-richlistitem");
+    is(listItemElems.length, mockHistory.length, "ensure result length");
+    is(itemsBox.mousedOverIndex, -1, "mousedOverIndex should be -1");
+
+    // navigate to the firt item
+    await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
+    is(autoCompletePopup.selectedIndex, 0, "selectedIndex should be 0");
+
+    // mouseover the second item
+    EventUtils.synthesizeMouseAtCenter(listItemElems[1], {type: "mouseover"});
+    await BrowserTestUtils.waitForCondition(() => {
+      return itemsBox.mousedOverIndex = 1;
+    });
+    ok(true, "mousedOverIndex changed");
+    is(autoCompletePopup.selectedIndex, 0, "selectedIndex should not be changed by mouseover");
+
+    // close popup
+    await ContentTask.spawn(browser, {}, async function() {
+      const input = content.document.querySelector("input");
+
+      input.blur();
+    });
+  });
+});
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -2585,17 +2585,24 @@ extends="chrome://global/content/binding
         while (item && item.localName != "richlistitem") {
           item = item.parentNode;
         }
 
         if (!item) {
           return;
         }
 
-        this.mousedOverIndex = this.getIndexOfItem(item);
+        let index = this.getIndexOfItem(item);
+
+        this.mousedOverIndex = index;
+
+        if (item.selectedByMouseOver) {
+          this.selectedIndex = index;
+        }
+
         this.mLastMoveTime = Date.now();
       ]]>
       </handler>
     </handlers>
   </binding>
 
   <binding id="autocomplete-treebody">
     <implementation>
--- a/toolkit/content/widgets/richlistbox.xml
+++ b/toolkit/content/widgets/richlistbox.xml
@@ -529,16 +529,18 @@
       <children/>
     </content>
 
     <resources>
       <stylesheet src="chrome://global/skin/richlistbox.css"/>
     </resources>
 
     <implementation>
+      <field name="selectedByMouseOver">false</field>
+
       <destructor>
         <![CDATA[
           var control = this.control;
           if (!control)
             return;
           // When we are destructed and we are current or selected, unselect ourselves
           // so that richlistbox's selection doesn't point to something not in the DOM.
           // We don't want to reset last-selected, so we set _suppressOnSelect.