Bug 1430001 - Highlight all used fonts in the rule-view, not just the first one; r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 12 Jan 2018 14:56:57 +0100
changeset 451080 f13e1bf54640cc830abab85a206a48c20cb20ff4
parent 451079 df10cf0252d0e8bd8f02aaa702bd9085a5c3bf89
child 451081 85e86d628948224426be069dfec817ceaf7d5316
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)
reviewersmiker
bugs1430001
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 1430001 - Highlight all used fonts in the rule-view, not just the first one; r=miker MozReview-Commit-ID: Gxi34noKsxu
devtools/client/inspector/rules/test/browser_rules_highlight-used-fonts.js
devtools/client/inspector/rules/views/text-property-editor.js
--- a/devtools/client/inspector/rules/test/browser_rules_highlight-used-fonts.js
+++ b/devtools/client/inspector/rules/test/browser_rules_highlight-used-fonts.js
@@ -18,41 +18,52 @@ const TEST_URI = `
       font-family: foo, monospace, monospace, serif;
     }
     #id4 {
       font-family: foo, bar;
     }
     #id5 {
       font-family: "monospace";
     }
+    #id6 {
+      font-family: georgia, arial;
+    }
   </style>
   <div id="id1">Text</div>
   <div id="id2">Text</div>
   <div id="id3">Text</div>
   <div id="id4">Text</div>
   <div id="id5">Text</div>
+  <div id="id6">A &#586;</div>
 `;
 
 // Tests that font-family properties in the rule-view correctly
 // indicates which font is in use.
 // Each entry in the test array should contain:
 // {
 //   selector: the rule-view selector to look for font-family in
 //   nb: the number of fonts this property should have
-//   used: the index of the font that should be highlighted, or
-//         -1 if none should be highlighted
+//   used: the indexes of all the fonts that should be highlighted, or null if none should
+//         be highlighter
 // }
 const TESTS = [
-  {selector: "#id1", nb: 3, used: 2}, // sans-serif
-  {selector: "#id2", nb: 1, used: 0}, // serif
-  {selector: "#id3", nb: 4, used: 1}, // monospace
-  {selector: "#id4", nb: 2, used: -1},
-  {selector: "#id5", nb: 1, used: 0}, // monospace
+  {selector: "#id1", nb: 3, used: [2]}, // sans-serif
+  {selector: "#id2", nb: 1, used: [0]}, // serif
+  {selector: "#id3", nb: 4, used: [1]}, // monospace
+  {selector: "#id4", nb: 2, used: null},
+  {selector: "#id5", nb: 1, used: [0]}, // monospace
 ];
 
+if (Services.appinfo.OS !== "Linux") {
+  // Both georgia and arial are used because the second character can't be rendered with
+  // georgia, so the browser falls back. Also, skip this on Linux which has neither of
+  // these fonts.
+  TESTS.push({selector: "#id6", nb: 2, used: [0, 1]});
+}
+
 add_task(function* () {
   yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
   let {inspector, view} = yield openRuleView();
 
   for (let {selector, nb, used} of TESTS) {
     let onFontHighlighted = view.once("font-highlighted");
     yield selectNode(selector, inspector);
     yield onFontHighlighted;
@@ -61,13 +72,20 @@ add_task(function* () {
 
     let prop = getRuleViewProperty(view, selector, "font-family").valueSpan;
     let fonts = prop.querySelectorAll(".ruleview-font-family");
 
     ok(fonts.length, "Fonts found in the property");
     is(fonts.length, nb, "Correct number of fonts found in the property");
 
     const highlighted = [...fonts].filter(span => span.classList.contains("used-font"));
+    const expectedHighlightedNb = used === null ? 0 : used.length;
+    is(highlighted.length, expectedHighlightedNb, "Correct number of used fonts found");
 
-    ok(highlighted.length <= 1, "No more than one font highlighted");
-    is([...fonts].findIndex(f => f === highlighted[0]), used, "Correct font highlighted");
+    let highlightedIndex = 0;
+    [...fonts].forEach((font, index) => {
+      if (font === highlighted[highlightedIndex]) {
+        is(index, used[highlightedIndex], "The right font is highlighted");
+        highlightedIndex++;
+      }
+    });
   }
 });
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -407,19 +407,16 @@ TextPropertyEditor.prototype = {
 
           if (!firstGenericSpan && GENERIC_FONT_FAMILIES.includes(authoredFont)) {
             firstGenericSpan = span;
           }
 
           if (usedFontFamilies.includes(authoredFont)) {
             span.classList.add("used-font");
             foundMatchingFamily = true;
-            // We found the span to style, no need to continue with
-            // the remaining ones
-            break;
           }
         }
 
         if (!foundMatchingFamily && firstGenericSpan) {
           firstGenericSpan.classList.add("used-font");
         }
 
         this.ruleView.emit("font-highlighted", this.valueSpan);