Bug 1186398, perform correct check when comparing if the select value has changed, r=felipe
authorNeil Deakin <neil@mozilla.com>
Mon, 27 Jul 2015 07:33:55 -0400
changeset 254764 effb4125322895c0c7f01d8bfe7ecb99f5c3a502
parent 254698 27ae736ef960c2f0f52ab289554cf6af3ed02721
child 254765 b44a29e2044c60f5409c56d5767d5a1ff2dd8108
push id16720
push userryanvm@gmail.com
push dateMon, 27 Jul 2015 19:45:38 +0000
treeherderb2g-inbound@13354b414396 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs1186398
milestone42.0a1
Bug 1186398, perform correct check when comparing if the select value has changed, r=felipe
browser/base/content/test/general/browser_selectpopup.js
toolkit/modules/SelectContentHelper.jsm
--- a/browser/base/content/test/general/browser_selectpopup.js
+++ b/browser/base/content/test/general/browser_selectpopup.js
@@ -1,52 +1,64 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // This test checks that a <select> with an <optgroup> opens and can be navigated
 // in a child process. This is different than single-process as a <menulist> is used
 // to implement the dropdown list.
 
 const PAGECONTENT =
-  "<html><body onload='document.body.firstChild.focus()'><select>" +
+  "<html><body onload='gChangeEvents = 0; document.body.firstChild.focus()'><select onchange='gChangeEvents++'>" +
   "  <optgroup label='First Group'>" +
   "    <option value=One>One" +
   "    <option value=Two>Two" +
   "  </optgroup>" +
   "  <option value=Three>Three" +
   "  <optgroup label='Second Group' disabled='true'>" +
   "    <option value=Four>Four" +
   "    <option value=Five>Five" +
   "  </optgroup>" +
   "  <option value=Six disabled='true'>Six" +
   "  <optgroup label='Third Group'>" +
   "    <option value=Seven>Seven" +
   "    <option value=Eight>Eight" +
-  "  </optgroup>" +
+  "  </optgroup></select><input>" +
   "</body></html>";
 
-function openSelectPopup(selectPopup)
+function openSelectPopup(selectPopup, withMouse)
 {
-  return new Promise((resolve, reject) => {
-    selectPopup.addEventListener("popupshown", function popupListener(event) {
-      selectPopup.removeEventListener("popupshown", popupListener, false)
-      resolve();
-    }, false);
-    setTimeout(() => EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true, code: "ArrowDown" }), 1500);
-  });
+  let popupShownPromise = BrowserTestUtils.waitForEvent(selectPopup, "popupshown");
+
+  if (withMouse) {
+    return Promise.all([popupShownPromise,
+                        BrowserTestUtils.synthesizeMouseAtCenter("select", { }, gBrowser.selectedBrowser)]);
+  }
+
+  setTimeout(() => EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true, code: "ArrowDown" }), 1500);
+  return popupShownPromise;
 }
 
-function hideSelectPopup(selectPopup)
+function hideSelectPopup(selectPopup, withEscape)
 {
-  return new Promise((resolve, reject) => {
-    selectPopup.addEventListener("popuphidden", function popupListener(event) {
-      selectPopup.removeEventListener("popuphidden", popupListener, false)
-      resolve();
-    }, false);
+  let popupShownPromise = BrowserTestUtils.waitForEvent(selectPopup, "popuphidden");
+
+  if (withEscape) {
+    EventUtils.synthesizeKey("KEY_Escape", { code: "Escape" });
+  }
+  else {
     EventUtils.synthesizeKey("KEY_Enter", { code: "Enter" });
+  }
+
+  return popupShownPromise;
+}
+
+function getChangeEvents()
+{
+  return ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
+    return content.wrappedJSObject.gChangeEvents;
   });
 }
 
 add_task(function*() {
   let tab = gBrowser.selectedTab = gBrowser.addTab();
   let browser = gBrowser.getBrowserForTab(tab);
   yield promiseTabLoadEvent(tab, "data:text/html," + escape(PAGECONTENT));
 
@@ -85,15 +97,34 @@ add_task(function*() {
   for (let i = 0; i < 10; i++) {
     is(menulist.getItemAtIndex(i).disabled, i >= 4 && i <= 7, "item " + i + " disabled")
   }
 
   EventUtils.synthesizeKey("KEY_ArrowUp", { code: "ArrowUp" });
   is(menulist.menuBoxObject.activeChild, menulist.getItemAtIndex(3), "Select item 3 again");
   is(menulist.selectedIndex, isWindows ? 3 : 1, "Select item 3 selectedIndex");
 
+  is((yield getChangeEvents()), 0, "Before closed - number of change events");
+
   yield hideSelectPopup(selectPopup);
 
   is(menulist.selectedIndex, 3, "Item 3 still selected");
+  is((yield getChangeEvents()), 1, "After closed - number of change events");
+
+  // Opening and closing the popup without changing the value should not fire a change event.
+  yield openSelectPopup(selectPopup, true);
+  yield hideSelectPopup(selectPopup, true);
+  is((yield getChangeEvents()), 1, "Open and close with no change - number of change events");
+  EventUtils.synthesizeKey("VK_TAB", { });
+  EventUtils.synthesizeKey("VK_TAB", { shiftKey: true });
+  is((yield getChangeEvents()), 1, "Tab away from select with no change - number of change events");
+
+  yield openSelectPopup(selectPopup, true);
+  EventUtils.synthesizeKey("KEY_ArrowDown", { code: "ArrowDown" });
+  yield hideSelectPopup(selectPopup, true);
+  is((yield getChangeEvents()), isWindows ? 2 : 1, "Open and close with change - number of change events");
+  EventUtils.synthesizeKey("VK_TAB", { });
+  EventUtils.synthesizeKey("VK_TAB", { shiftKey: true });
+  is((yield getChangeEvents()), isWindows ? 2 : 1, "Tab away from select with change - number of change events");
 
   gBrowser.removeCurrentTab();
 });
 
--- a/toolkit/modules/SelectContentHelper.jsm
+++ b/toolkit/modules/SelectContentHelper.jsm
@@ -61,17 +61,17 @@ this.SelectContentHelper.prototype = {
 
   receiveMessage: function(message) {
     switch (message.name) {
       case "Forms:SelectDropDownItem":
         this.element.selectedIndex = message.data.value;
         break;
 
       case "Forms:DismissedDropDown":
-        if (this.initialSelection != this.element.item[this.element.selectedIndex]) {
+        if (this.initialSelection != this.element.item(this.element.selectedIndex)) {
           let event = this.element.ownerDocument.createEvent("Events");
           event.initEvent("change", true, true);
           this.element.dispatchEvent(event);
         }
 
         this.uninit();
         break;
     }