Bug 1300996 - Part 1: Set selectedIndex of autocomplete popup when mousemove over profile item. r=adw
☠☠ backed out by ac7da51d8ccc ☠ ☠
authorRay Lin <ralin@mozilla.com>
Tue, 25 Apr 2017 16:48:52 +0800
changeset 410780 f54fca58d189cfc259766817faaa3a962ad96933
parent 410779 77dc3e0df3b68bb617ba4474d679aaa54a44bd17
child 410781 da12ceebe125788134a169e5fac186685423dd2b
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.