Bug 1429672 - ensure DeferredTask is defined r=jaws
authorMyk Melez <myk@mykzilla.org>
Thu, 11 Jan 2018 12:26:35 -0800
changeset 450758 72823606d8b47ddbbfa7cb2dd975e0e52f4a60a3
parent 450757 ce4d5812e03a48bc5e217b721bea750d43577a33
child 450759 fa7d09e322e37059b8ace0ffff8a92b00b93b0ff
push id8543
push userryanvm@gmail.com
push dateTue, 16 Jan 2018 14:33:22 +0000
treeherdermozilla-beta@a6525ed16a32 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1429672
milestone59.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 1429672 - ensure DeferredTask is defined r=jaws Binding the Preferences object to a "this" object was supposed to enable userChangedValue to access the DeferredTask instance that XPCOMUtils.defineLazyModuleGetter defines on the "this" object. I thought that worked, because tests passed, but it turned out that no tests are exercising the branch within userChangedValue that references DeferredTask, which is only followed for pref form controls that specify "delayprefsave." Only font controls specify that attribute, and there weren't any tests for changes to those controls. This patch adds such tests and then changes the way that DeferredTask is lazily retrieved and referenced to ensure it's defined as referenced in userChangedValue. MozReview-Commit-ID: BPbGp55s9wC
browser/components/preferences/in-content/tests/browser_basic_rebuild_fonts_test.js
toolkit/content/preferencesBindings.js
--- a/browser/components/preferences/in-content/tests/browser_basic_rebuild_fonts_test.js
+++ b/browser/components/preferences/in-content/tests/browser_basic_rebuild_fonts_test.js
@@ -12,20 +12,70 @@ add_task(async function() {
   let doc = gBrowser.contentDocument;
   // eslint-disable-next-line mozilla/no-cpows-in-tests
   let contentWindow = gBrowser.contentWindow;
   var langGroup = Services.prefs.getComplexValue("font.language.group", Ci.nsIPrefLocalizedString).data;
   is(contentWindow.Preferences.get("font.language.group").value, langGroup,
      "Language group should be set correctly.");
 
   let defaultFontType = Services.prefs.getCharPref("font.default." + langGroup);
-  let fontFamily = Services.prefs.getCharPref("font.name." + defaultFontType + "." + langGroup);
+  let fontFamilyPref = "font.name." + defaultFontType + "." + langGroup;
+  let fontFamily = Services.prefs.getCharPref(fontFamilyPref);
   let fontFamilyField = doc.getElementById("defaultFont");
   is(fontFamilyField.value, fontFamily, "Font family should be set correctly.");
 
+  function dispatchMenuItemCommand(menuItem) {
+    const cmdEvent = doc.createEvent("xulcommandevent");
+    cmdEvent.initCommandEvent("command", true, true, contentWindow, 0, false, false, false, false, null, 0);
+    menuItem.dispatchEvent(cmdEvent);
+  }
+
+  /**
+   * Return a promise that resolves when the fontFamilyPref changes.
+   *
+   * Font prefs are the only ones whose form controls set "delayprefsave",
+   * which delays the pref change when a user specifies a new value
+   * for the pref.  Thus, in order to confirm that the pref gets changed
+   * when the test selects a new value in a font field, we need to await
+   * the change.  Awaiting this function does so for fontFamilyPref.
+   */
+  function fontFamilyPrefChanged() {
+    return new Promise(resolve => {
+      const observer = {
+        observe(aSubject, aTopic, aData) {
+          // Check for an exact match to avoid the ambiguity of nsIPrefBranch's
+          // prefix-matching algorithm for notifying pref observers.
+          if (aData == fontFamilyPref) {
+            Services.prefs.removeObserver(fontFamilyPref, observer);
+            resolve();
+          }
+        }
+      };
+      Services.prefs.addObserver(fontFamilyPref, observer);
+    });
+  }
+
+  const menuItems = fontFamilyField.querySelectorAll("menuitem");
+  ok(menuItems.length > 1, "There are multiple font menuitems.");
+  ok(menuItems[0].selected, "The first (default) font menuitem is selected.");
+
+  dispatchMenuItemCommand(menuItems[1]);
+  ok(menuItems[1].selected, "The second font menuitem is selected.");
+
+  await fontFamilyPrefChanged();
+  fontFamily = Services.prefs.getCharPref(fontFamilyPref);
+  is(fontFamilyField.value, fontFamily, "The font family has been updated.");
+
+  dispatchMenuItemCommand(menuItems[0]);
+  ok(menuItems[0].selected, "The first (default) font menuitem is selected again.");
+
+  await fontFamilyPrefChanged();
+  fontFamily = Services.prefs.getCharPref(fontFamilyPref);
+  is(fontFamilyField.value, fontFamily, "The font family has been updated.");
+
   let defaultFontSize = Services.prefs.getIntPref("font.size.variable." + langGroup);
   let fontSizeField = doc.getElementById("defaultFontSize");
   is(fontSizeField.value, defaultFontSize, "Font size should be set correctly.");
 
   let promiseSubDialogLoaded = promiseLoadSubDialog("chrome://browser/content/preferences/fonts.xul");
   doc.getElementById("advancedFonts").click();
   let win = await promiseSubDialogLoaded;
   doc = win.document;
--- a/toolkit/content/preferencesBindings.js
+++ b/toolkit/content/preferencesBindings.js
@@ -8,17 +8,18 @@
 // have access to it.
 const Preferences = window.Preferences = (function() {
   const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
   Cu.import("resource://gre/modules/EventEmitter.jsm");
   Cu.import("resource://gre/modules/Services.jsm");
   Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
-  XPCOMUtils.defineLazyModuleGetter(this, "DeferredTask",
+  const lazy = {};
+  XPCOMUtils.defineLazyModuleGetter(lazy, "DeferredTask",
                                     "resource://gre/modules/DeferredTask.jsm");
 
   function getElementsByAttribute(name, value) {
     // If we needed to defend against arbitrary values, we would escape
     // double quotes (") and escape characters (\) in them, i.e.:
     //   ${value.replace(/["\\]/g, '\\$&')}
     return value ? document.querySelectorAll(`[${name}="${value}"]`)
                  : document.querySelectorAll(`[${name}]`);
@@ -175,17 +176,17 @@ const Preferences = window.Preferences =
       if (element.hasAttribute("preference")) {
         if (element.getAttribute("delayprefsave") != "true") {
           const preference = Preferences.get(element.getAttribute("preference"));
           const prefVal = preference.getElementValue(element);
           preference.value = prefVal;
         } else {
           if (!element._deferredValueUpdateTask) {
             element._deferredValueUpdateTask =
-              new DeferredTask(this._deferredValueUpdate.bind(this, element), 1000);
+              new lazy.DeferredTask(this._deferredValueUpdate.bind(this, element), 1000);
             this._deferredValueUpdateElements.add(element);
           } else {
             // Each time the preference is changed, restart the delay.
             element._deferredValueUpdateTask.disarm();
           }
           element._deferredValueUpdateTask.arm();
         }
       }
@@ -608,9 +609,9 @@ const Preferences = window.Preferences =
       if (!this.batching) {
         Services.prefs.savePrefFile(null);
       }
       return val;
     }
   }
 
   return Preferences;
-}.bind({})());
+}());