Bug 1473263 - Show "No fonts" message when there are no fonts used on the selected element or its descendants. r=pbro draft
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 04 Jul 2018 18:39:06 +0200
changeset 814415 164d76ee72792a0e826d9aea03495cf45319d76f
parent 814369 90be04d99fc7941cb9b7186bf5f95e184a4e989a
push id115194
push userbmo:rcaliman@mozilla.com
push dateThu, 05 Jul 2018 08:58:04 +0000
reviewerspbro
bugs1473263
milestone63.0a1
Bug 1473263 - Show "No fonts" message when there are no fonts used on the selected element or its descendants. r=pbro MozReview-Commit-ID: Bkgs6ulw8nD
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontOverview.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector.html
devtools/client/inspector/fonts/test/browser_fontinspector_no-fonts.js
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -277,19 +277,24 @@ class FontEditor extends PureComponent {
       ),
       instanceSelect
     );
   }
 
   renderWarning() {
     return dom.div(
       {
-        className: "devtools-sidepanel-no-result"
+        id: "font-editor"
       },
-      getStr("fontinspector.noFontsOnSelectedElement")
+      dom.div(
+        {
+          className: "devtools-sidepanel-no-result"
+        },
+        getStr("fontinspector.noFontsOnSelectedElement")
+      )
     );
   }
 
   render() {
     const { fontEditor } = this.props;
     const { fonts, families, axes, instance, properties } = fontEditor;
     // Pick the first font to show editor controls regardless of how many fonts are used.
     const font = fonts[0];
@@ -297,25 +302,26 @@ class FontEditor extends PureComponent {
     const hasFontInstances = font && font.variationInstances
       && font.variationInstances.length > 0;
     const hasSlantOrItalicAxis = hasFontAxes && font.variationAxes.find(axis => {
       return axis.tag === "slnt" || axis.tag === "ital";
     });
     const hasWeightAxis = hasFontAxes && font.variationAxes.find(axis => {
       return axis.tag === "wght";
     });
-    // Check for falsy font-weight value (undefined or empty string).
-    const hasWeight = properties["font-weight"] != null;
+
+    // Render empty state message for nodes that don't have a used font.
+    if (!font) {
+      return this.renderWarning();
+    }
 
     return dom.div(
       {
         id: "font-editor"
       },
-      // Render empty state message for nodes that don't have font properties.
-      !hasWeight && this.renderWarning(),
       // Always render UI for font family, format and font file URL.
       this.renderFontFamily(fonts, families),
       // Render UI for font variation instances if they are defined.
       hasFontInstances && this.renderInstances(font.variationInstances, instance),
       // Always render UI for font size.
       this.renderFontSize(properties["font-size"]),
       // Render UI for font weight if no "wght" registered axis is defined.
       !hasWeightAxis && this.renderFontWeight(properties["font-weight"]),
--- a/devtools/client/inspector/fonts/components/FontOverview.js
+++ b/devtools/client/inspector/fonts/components/FontOverview.js
@@ -41,33 +41,34 @@ class FontOverview extends PureComponent
       onPreviewFonts,
       onToggleFontHighlight,
     } = this.props;
     const { fonts } = fontData;
 
     // If the font editor is enabled, show the fonts in a collapsed accordion.
     // The editor already displays fonts, in another way, rendering twice is not desired.
     if (Services.prefs.getBoolPref(PREF_FONT_EDITOR)) {
-      return fonts.length ? Accordion({
-        items: [
-          {
-            header: getStr("fontinspector.renderedFontsInPageHeader"),
-            component: FontList,
-            componentProps: {
-              fonts,
-              fontOptions,
-              onPreviewFonts,
-              onToggleFontHighlight,
-            },
-            opened: false
-          }
-        ]
-      })
-      :
-      null;
+      return fonts.length ?
+        Accordion({
+          items: [
+            {
+              header: getStr("fontinspector.renderedFontsInPageHeader"),
+              component: FontList,
+              componentProps: {
+                fonts,
+                fontOptions,
+                onPreviewFonts,
+                onToggleFontHighlight,
+              },
+              opened: false
+            }
+          ]
+        })
+        :
+        null;
     }
 
     return fonts.length ?
       FontList({
         fonts,
         fontOptions,
         onPreviewFonts,
         onToggleFontHighlight,
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -680,18 +680,19 @@ class FontInspector {
     const node = this.inspector.selection.nodeFront;
     const fonts = await this.getFontsForNode(node, options);
 
     // Get computed styles for the selected node, but filter by CSS font properties.
     this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
       filterProperties: FONT_PROPERTIES
     });
 
-    if (!this.nodeComputedStyle) {
+    if (!this.nodeComputedStyle || !fonts.length) {
       this.store.dispatch(resetFontEditor());
+      this.inspector.emit("fonteditor-updated");
       return;
     }
 
     // Clear any references to writer methods and CSS declarations because the node's
     // styles may have changed since the last font editor refresh.
     this.writers.clear();
 
     // If the Rule panel is not visible, the selected element's rule models may not have
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -19,12 +19,13 @@ support-files =
 [browser_fontinspector_copy-URL.js]
 skip-if = !e10s # too slow on !e10s, logging fully serialized actors (Bug 1446595)
 subsuite = clipboard
 [browser_fontinspector_edit-previews.js]
 [browser_fontinspector_editor-values.js]
 [browser_fontinspector_editor-keywords.js]
 [browser_fontinspector_expand-css-code.js]
 [browser_fontinspector_family-unused.js]
+[browser_fontinspector_no-fonts.js]
 [browser_fontinspector_other-fonts.js]
 [browser_fontinspector_rendered-fonts.js]
 [browser_fontinspector_reveal-in-page.js]
 [browser_fontinspector_theme-change.js]
--- a/devtools/client/inspector/fonts/test/browser_fontinspector.html
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector.html
@@ -48,9 +48,10 @@
   BODY
   <div>DIV
     <span class="nested-span">NESTED SPAN</span>
   </div>
   <iframe src="test_iframe.html"></iframe>
   <div class="normal-text">NORMAL DIV</div>
   <div class="bold-text">BOLD DIV</div>
   <div class="black-text">800 DIV</div>
+  <div class="empty"></div>
 </body>
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_no-fonts.js
@@ -0,0 +1,25 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+// Check that the warning message for no fonts found shows up when selecting a node
+// that does not have any used fonts.
+// Ensure that no used fonts are listed.
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+add_task(async function() {
+  await pushPref("devtools.inspector.fonteditor.enabled", true);
+  const { view, inspector } = await openFontInspectorForURL(TEST_URI);
+  const viewDoc = view.document;
+  await selectNode(".empty", inspector);
+
+  info("Test the warning message for no fonts found on empty element");
+  const warning = viewDoc.querySelector("#font-editor .devtools-sidepanel-no-result");
+  ok(warning, "The warning for no fonts found is shown for the empty element");
+
+  info("Test that no fonts are listed for the empty element");
+  const fontsEls = getUsedFontsEls(viewDoc);
+  is(fontsEls.length, 0, "There are no used fonts listed");
+});