Bug 1300996 - Part 1: Set selectedIndex of autocomplete popup when mousemove over profile item. r=adw draft
authorRay Lin <ralin@mozilla.com>
Tue, 25 Apr 2017 16:48:52 +0800
changeset 590888 e0af703a4a6291c53995fd00319bf8a6b259109c
parent 589954 5801aa478de12a62b2b2982659e787fcc4268d67
child 590889 b7e4f4f9f1bb8fc356f6abf2863cd1d7f6159a61
child 591433 7394e1e0407829fcc4604d764e8cfd8afc326ef3
push id62865
push userbmo:ralin@mozilla.com
push dateThu, 08 Jun 2017 08:21:20 +0000
reviewersadw
bugs1300996
milestone55.0a1
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.