Bug 1344990 part.4 FontBuilder.readFontSelection() should return empty string when saved font isn't available r=jaws
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 29 Mar 2017 23:55:37 +0900
changeset 555293 0b45ef5f7702377bded5a67c6af0a713eccfc41e
parent 555292 74fa1a2c624020caa50d8c13028268550defc537
child 555294 0aeb3c980745760195ccffad31627eec11a663cf
push id52211
push userbmo:jaws@mozilla.com
push dateTue, 04 Apr 2017 01:40:08 +0000
reviewersjaws
bugs1344990
milestone55.0a1
Bug 1344990 part.4 FontBuilder.readFontSelection() should return empty string when saved font isn't available r=jaws FontBuilder adjusted font settings if reading font isn't available in the system. Now, we support empty string value of "font.name.*" which means default font in the system. If it's empty string, gfx checks default font from "font.name-list.*" at runtime. Therefore, resetting "font.name.*" to empty string is the best approach if specified font becomes not available (e.g., uninstalled from the system). Therefore, this patch replaces the code in FontBuilder.readFontSelection() with just returning empty string. MozReview-Commit-ID: HgR88Qw8Xwe
browser/components/preferences/in-content-old/tests/browser_basic_rebuild_fonts_test.js
browser/components/preferences/in-content/tests/browser_basic_rebuild_fonts_test.js
toolkit/mozapps/preferences/fontbuilder.js
--- a/browser/components/preferences/in-content-old/tests/browser_basic_rebuild_fonts_test.js
+++ b/browser/components/preferences/in-content-old/tests/browser_basic_rebuild_fonts_test.js
@@ -22,55 +22,88 @@ add_task(function*() {
 
   doc.getElementById("advancedFonts").click();
   let win = yield promiseLoadSubDialog("chrome://browser/content/preferences/fonts.xul");
   doc = win.document;
 
   // Simulate a dumb font backend.
   win.FontBuilder._enumerator = {
     _list: ["MockedFont1", "MockedFont2", "MockedFont3"],
+    _defaultFont: null,
     EnumerateFonts(lang, type, list) {
       return this._list;
     },
     EnumerateAllFonts() {
       return this._list;
     },
-    getDefaultFont() { return null; },
+    getDefaultFont() { return this._defaultFont; },
     getStandardFamilyName(name) { return name; },
   };
   win.FontBuilder._allFonts = null;
   win.FontBuilder._langGroupSupported = false;
 
   let langGroupElement = doc.getElementById("font.language.group");
   let selectLangsField = doc.getElementById("selectLangs");
   let serifField = doc.getElementById("serif");
   let armenian = "x-armn";
   let western = "x-western";
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
   is(serifField.value, "", "Font family should not be set.");
 
+  let armenianSerifElement = doc.getElementById("font.name.serif.x-armn");
+
   langGroupElement.value = western;
   selectLangsField.value = western;
 
   // Simulate a font backend supporting language-specific enumeration.
   // NB: FontBuilder has cached the return value from EnumerateAllFonts(),
   // so _allFonts will always have 3 elements regardless of subsequent
   // _list changes.
   win.FontBuilder._enumerator._list = ["MockedFont2"];
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
-  is(serifField.value, "MockedFont2", "Font family should be set.");
+  is(serifField.value, "", "Font family should still be empty for indicating using 'default' font.");
 
   langGroupElement.value = western;
   selectLangsField.value = western;
 
   // Simulate a system that has no fonts for the specified language.
   win.FontBuilder._enumerator._list = [];
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
   is(serifField.value, "", "Font family should not be set.");
 
+  // Setting default font to "MockedFont3".  Then, when serifField.value is
+  // empty, it should indicate using "MockedFont3" but it shouldn't be saved
+  // to "MockedFont3" in the pref.  It should be resolved at runtime.
+  win.FontBuilder._enumerator._list = ["MockedFont1", "MockedFont2", "MockedFont3"];
+  win.FontBuilder._enumerator._defaultFont = "MockedFont3";
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "", "Font family should be empty even if there is a default font.");
+
+  armenianSerifElement.value = "MockedFont2";
+  serifField.value = "MockedFont2";
+  is(serifField.value, "MockedFont2", "Font family should be \"MockedFont2\" for now.");
+
+  langGroupElement.value = western;
+  selectLangsField.value = western;
+  is(serifField.value, "", "Font family of other language should not be set.");
+
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "MockedFont2", "Font family should not be changed even after switching the language.");
+
+  // If MochedFont2 is removed from the system, the value should be treated
+  // as empty (i.e., 'default' font) after rebuilding the font list.
+  win.FontBuilder._enumerator._list = ["MockedFont1", "MockedFont3"];
+  win.FontBuilder._enumerator._allFonts = ["MockedFont1", "MockedFont3"];
+  serifField.removeAllItems(); // This will cause rebuilding the font list from available fonts.
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "", "Font family should become empty due to the font uninstalled.");
+
   gBrowser.removeCurrentTab();
 });
--- 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
@@ -22,55 +22,88 @@ add_task(function*() {
 
   doc.getElementById("advancedFonts").click();
   let win = yield promiseLoadSubDialog("chrome://browser/content/preferences/fonts.xul");
   doc = win.document;
 
   // Simulate a dumb font backend.
   win.FontBuilder._enumerator = {
     _list: ["MockedFont1", "MockedFont2", "MockedFont3"],
+    _defaultFont: null,
     EnumerateFonts(lang, type, list) {
       return this._list;
     },
     EnumerateAllFonts() {
       return this._list;
     },
-    getDefaultFont() { return null; },
+    getDefaultFont() { return this._defaultFont; },
     getStandardFamilyName(name) { return name; },
   };
   win.FontBuilder._allFonts = null;
   win.FontBuilder._langGroupSupported = false;
 
   let langGroupElement = doc.getElementById("font.language.group");
   let selectLangsField = doc.getElementById("selectLangs");
   let serifField = doc.getElementById("serif");
   let armenian = "x-armn";
   let western = "x-western";
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
   is(serifField.value, "", "Font family should not be set.");
 
+  let armenianSerifElement = doc.getElementById("font.name.serif.x-armn");
+
   langGroupElement.value = western;
   selectLangsField.value = western;
 
   // Simulate a font backend supporting language-specific enumeration.
   // NB: FontBuilder has cached the return value from EnumerateAllFonts(),
   // so _allFonts will always have 3 elements regardless of subsequent
   // _list changes.
   win.FontBuilder._enumerator._list = ["MockedFont2"];
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
-  is(serifField.value, "MockedFont2", "Font family should be set.");
+  is(serifField.value, "", "Font family should still be empty for indicating using 'default' font.");
 
   langGroupElement.value = western;
   selectLangsField.value = western;
 
   // Simulate a system that has no fonts for the specified language.
   win.FontBuilder._enumerator._list = [];
 
   langGroupElement.value = armenian;
   selectLangsField.value = armenian;
   is(serifField.value, "", "Font family should not be set.");
 
+  // Setting default font to "MockedFont3".  Then, when serifField.value is
+  // empty, it should indicate using "MockedFont3" but it shouldn't be saved
+  // to "MockedFont3" in the pref.  It should be resolved at runtime.
+  win.FontBuilder._enumerator._list = ["MockedFont1", "MockedFont2", "MockedFont3"];
+  win.FontBuilder._enumerator._defaultFont = "MockedFont3";
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "", "Font family should be empty even if there is a default font.");
+
+  armenianSerifElement.value = "MockedFont2";
+  serifField.value = "MockedFont2";
+  is(serifField.value, "MockedFont2", "Font family should be \"MockedFont2\" for now.");
+
+  langGroupElement.value = western;
+  selectLangsField.value = western;
+  is(serifField.value, "", "Font family of other language should not be set.");
+
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "MockedFont2", "Font family should not be changed even after switching the language.");
+
+  // If MochedFont2 is removed from the system, the value should be treated
+  // as empty (i.e., 'default' font) after rebuilding the font list.
+  win.FontBuilder._enumerator._list = ["MockedFont1", "MockedFont3"];
+  win.FontBuilder._enumerator._allFonts = ["MockedFont1", "MockedFont3"];
+  serifField.removeAllItems(); // This will cause rebuilding the font list from available fonts.
+  langGroupElement.value = armenian;
+  selectLangsField.value = armenian;
+  is(serifField.value, "", "Font family should become empty due to the font uninstalled.");
+
   gBrowser.removeCurrentTab();
 });
--- a/toolkit/mozapps/preferences/fontbuilder.js
+++ b/toolkit/mozapps/preferences/fontbuilder.js
@@ -95,28 +95,14 @@ var FontBuilder = {
     if (preference.value) {
       let fontItems = aElement.getElementsByAttribute("value", preference.value);
 
       // There is a setting that actually is in the list. Respect it.
       if (fontItems.length)
         return undefined;
     }
 
-    // The first item will be a reasonable choice only if the font backend
-    // supports language-specific enumaration.
-    let defaultValue = this._langGroupSupported ?
-                       aElement.firstChild.firstChild.getAttribute("value") : "";
-    let fontNameList = preference.name.replace(".name.", ".name-list.");
-    let prefFontNameList = document.getElementById(fontNameList);
-    if (!prefFontNameList || !prefFontNameList.value)
-      return defaultValue;
-
-    let fontNames = prefFontNameList.value.split(",");
-
-    for (let i = 0; i < fontNames.length; ++i) {
-      let fontName = this.enumerator.getStandardFamilyName(fontNames[i].trim());
-      let fontItems = aElement.getElementsByAttribute("value", fontName);
-      if (fontItems.length)
-        return fontItems[0].getAttribute("value");
-    }
-    return defaultValue;
+    // Otherwise, use "default" font of current system which is computed
+    // with "font.name-list.*".  If "font.name.*" is empty string, it means
+    // "default".  So, return empty string in this case.
+    return "";
   }
 };