Bug 1472116 - Show the font overview in a collapsible section in the font editor; r=rcaliman draft
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 02 Jul 2018 14:29:34 +0200
changeset 813094 70fcaf0f7cfcb65ac0cd142610ec2b48306c9ac9
parent 812962 3cfc350101967376909ad3c729f9779ae0ab7a94
push id114769
push userbmo:pbrosset@mozilla.com
push dateMon, 02 Jul 2018 12:33:10 +0000
reviewersrcaliman
bugs1472116
milestone63.0a1
Bug 1472116 - Show the font overview in a collapsible section in the font editor; r=rcaliman MozReview-Commit-ID: AcA1jkGKCW9
devtools/client/inspector/fonts/components/FontOverview.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector_other-fonts.js
devtools/client/inspector/fonts/test/browser_fontinspector_rendered-fonts.js
devtools/client/inspector/fonts/test/head.js
devtools/client/locales/en-US/font-inspector.properties
--- a/devtools/client/inspector/fonts/components/FontOverview.js
+++ b/devtools/client/inspector/fonts/components/FontOverview.js
@@ -30,30 +30,44 @@ class FontOverview extends PureComponent
   constructor(props) {
     super(props);
     this.onToggleFontHighlightGlobal = (font, show) => {
       this.props.onToggleFontHighlight(font, show, false);
     };
   }
 
   renderElementFonts() {
-    // Do not show element fonts if the font editor is enabled.
-    // It handles this differently. Rendering twice is not desired.
-    if (Services.prefs.getBoolPref(PREF_FONT_EDITOR)) {
-      return null;
-    }
-
     const {
       fontData,
       fontOptions,
       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 Accordion({
+        items: [
+          {
+            header: getStr("fontinspector.renderedFontsInPageHeader"),
+            component: FontList,
+            componentProps: {
+              fonts,
+              fontOptions,
+              onPreviewFonts,
+              onToggleFontHighlight,
+            },
+            opened: false
+          }
+        ]
+      });
+    }
+
     return fonts.length ?
       FontList({
         fonts,
         fontOptions,
         onPreviewFonts,
         onToggleFontHighlight,
       })
       :
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -20,10 +20,11 @@ support-files =
 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_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_other-fonts.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_other-fonts.js
@@ -10,21 +10,21 @@
 
 const TEST_URI = URL_ROOT + "browser_fontinspector.html";
 
 add_task(async function() {
   await pushPref("devtools.inspector.fonteditor.enabled", true);
   const { inspector, view } = await openFontInspectorForURL(TEST_URI);
   const viewDoc = view.document;
 
-  const otherFontsAccordion = viewDoc.querySelector("#font-container .accordion");
+  const otherFontsAccordion = getOtherFontsAccordion(viewDoc);
   ok(otherFontsAccordion, "There's an accordion in the panel");
   is(otherFontsAccordion.textContent, "Other fonts in page", "It has the right title");
 
-  await expandOtherFontsAccordion(viewDoc);
+  await expandAccordion(otherFontsAccordion);
   let otherFontsEls = getOtherFontsEls(viewDoc);
 
   is(otherFontsEls.length, 1, "There is one font listed in the other fonts section");
   // On Linux Times New Roman does not exist, Liberation Serif is used instead
   const name = getName(otherFontsEls[0]);
   ok(name === "Times New Roman" || name === "Liberation Serif",
      "The other font listed is the right one");
 
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_rendered-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 font inspector has a section for "rendered fonts" and that section
+// contains the fonts used to render the content of the selected element.
+// Check that it's collapsed by default, but can be expanded.
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+add_task(async function() {
+  await pushPref("devtools.inspector.fonteditor.enabled", true);
+  const { view } = await openFontInspectorForURL(TEST_URI);
+  const viewDoc = view.document;
+
+  const renderedFontsAccordion = getRenderedFontsAccordion(viewDoc);
+  ok(renderedFontsAccordion, "There's an accordion for rendered fonts in the panel");
+  is(renderedFontsAccordion.textContent, "Rendered fonts", "It has the right title");
+
+  await expandAccordion(renderedFontsAccordion);
+  const fontsEls = getRenderedFontsEls(viewDoc);
+
+  ok(fontsEls.length, "There are fonts listed in the rendered fonts section");
+});
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -96,42 +96,82 @@ async function updatePreviewText(view, t
  * @param  {document} viewDoc
  * @return {Array}
  */
 function getUsedFontsEls(viewDoc) {
   return viewDoc.querySelectorAll("#font-editor .fonts-list li");
 }
 
 /**
- * Expand the other fonts accordion.
+ * Get the DOM element for the accordion widget that contains the fonts used to render the
+ * current element.
+ *
+ * @param  {document} viewDoc
+ * @return {DOMNode}
  */
-async function expandOtherFontsAccordion(viewDoc) {
-  info("Expanding the other fonts section");
+function getRenderedFontsAccordion(viewDoc) {
+  return viewDoc.querySelectorAll("#font-container .accordion")[0];
+}
 
-  const accordion = viewDoc.querySelector("#font-container .accordion");
+/**
+ * Get the DOM element for the accordion widget that contains the fonts used elsewhere in
+ * the document.
+ *
+ * @param  {document} viewDoc
+ * @return {DOMNode}
+ */
+function getOtherFontsAccordion(viewDoc) {
+  return viewDoc.querySelectorAll("#font-container .accordion")[1];
+}
+
+/**
+ * Expand a given accordion widget.
+ *
+ * @param  {DOMNode} accordion
+ */
+async function expandAccordion(accordion) {
   const isExpanded = () => accordion.querySelector(".fonts-list");
-
   if (isExpanded()) {
     return;
   }
 
-  const onExpanded = BrowserTestUtils.waitForCondition(isExpanded,
-                                                     "Waiting for other fonts section");
+  const onExpanded = BrowserTestUtils.waitForCondition(
+    isExpanded, "Waiting for other fonts section");
   accordion.querySelector(".theme-twisty").click();
   await onExpanded;
 }
 
 /**
+ * Expand the other fonts accordion.
+ *
+ * @param  {document} viewDoc
+ */
+async function expandOtherFontsAccordion(viewDoc) {
+  info("Expanding the other fonts section");
+  await expandAccordion(getOtherFontsAccordion(viewDoc));
+}
+
+/**
+ * Get all of the <li> elements for the fonts used to render the current element.
+ *
+ * @param  {document} viewDoc
+ * @return {Array}
+ */
+function getRenderedFontsEls(viewDoc) {
+  return getRenderedFontsAccordion(viewDoc).querySelectorAll(".fonts-list > li");
+}
+
+/**
  * Get all of the <li> elements for the fonts used elsewhere in the document.
  *
  * @param  {document} viewDoc
  * @return {Array}
  */
 function getOtherFontsEls(viewDoc) {
-  return viewDoc.querySelectorAll("#font-container .accordion .fonts-list > li");
+  return getOtherFontsAccordion(viewDoc).querySelectorAll(".fonts-list > li");
 }
 
 /**
  * Given a font element, return its name.
  *
  * @param  {DOMNode} fontEl
  *         The font element.
  * @return {String}
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -12,16 +12,21 @@ fontinspector.system=system
 # LOCALIZATION NOTE (fontinspector.noFontsOnSelectedElement): This label is shown when
 # no fonts found on the selected element.
 fontinspector.noFontsOnSelectedElement=No fonts were found for the current element.
 
 # LOCALIZATION NOTE (fontinspector.otherFontsInPageHeader): This is the text for the
 # header of a collapsible section containing other fonts used in the page.
 fontinspector.otherFontsInPageHeader=Other fonts in page
 
+# LOCALIZATION NOTE (fontinspector.renderedFontsInPageHeader): This is the text for the
+# header of a collapsible section containing the fonts used to render the content of the
+# selected element.
+fontinspector.renderedFontsInPageHeader=Rendered fonts
+
 # LOCALIZATION NOTE (fontinspector.editPreview): This is the text that appears in a
 # tooltip on hover of a font preview string. Clicking on the string opens a text input
 # where users can type to change the preview text.
 fontinspector.editPreview=Click to edit preview
 
 # LOCALIZATION NOTE (fontinspector.copyURL): This is the text that appears in a tooltip
 # displayed when the user hovers over the copy icon next to the font URL.
 # Clicking the copy icon copies the full font URL to the user's clipboard