Bug 1485324 - (Part 1) Remove pref for Font Editor and obsolete condi…

…tionals; r=gl This patch removes the `devtools.inspector.fonteditor.enabled` pref and all its uses in the Fonts panel. Obsolete actions for the case when the pref was off are also removed. This is mostly old Font Inspector code. One test is temporarily disabled because it tests the old Font Inspector. It will be removed along with other pieces on the next part of this commit series. Differential Revision: https://phabricator.services.mozilla.com/D11505 --HG-- extra : moz-landing-system : lando
dothq · Nov 13, 2018 · d5f2881 · d5f2881
1 parent b476025
commit d5f2881
Show file tree

Hide file tree

Showing 20 changed files with 11 additions and 89 deletions.
diff --git a/devtools/client/inspector/fonts/components/FontList.js b/devtools/client/inspector/fonts/components/FontList.js
@@ -4,7 +4,6 @@
 
 "use strict";
 
-const Services = require("Services");
 const {
   createElement,
   createFactory,
@@ -20,8 +19,6 @@ const FontPreviewInput = createFactory(require("./FontPreviewInput"));
 
 const Types = require("../types");
 
-const PREF_FONT_EDITOR = "devtools.inspector.fonteditor.enabled";
-
 class FontList extends PureComponent {
   static get propTypes() {
     return {
@@ -70,15 +67,11 @@ class FontList extends PureComponent {
       }))
     );
 
-    // Show the font preview input only when the font editor is enabled.
-    const previewInput = Services.prefs.getBoolPref(PREF_FONT_EDITOR) ?
-      FontPreviewInput({
-        ref: this.previewInputRef,
-        onPreviewTextChange,
-        previewText,
-      })
-      :
-      null;
+    const previewInput = FontPreviewInput({
+      ref: this.previewInputRef,
+      onPreviewTextChange,
+      previewText,
+    });
 
     return createElement(Fragment, null, previewInput, list);
   }

diff --git a/devtools/client/inspector/fonts/components/FontOverview.js b/devtools/client/inspector/fonts/components/FontOverview.js
@@ -7,16 +7,13 @@
 const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const Services = require("Services");
 
 const Accordion = createFactory(require("devtools/client/inspector/layout/components/Accordion"));
 const FontList = createFactory(require("./FontList"));
 
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
-const PREF_FONT_EDITOR = "devtools.inspector.fonteditor.enabled";
-
 class FontOverview extends PureComponent {
   static get propTypes() {
     return {
@@ -34,45 +31,14 @@ class FontOverview extends PureComponent {
     };
   }
 
-  renderElementFonts() {
-    const {
-      fontData,
-      fontOptions,
-      onPreviewTextChange,
-      onToggleFontHighlight,
-    } = this.props;
-    const { fonts } = fontData;
-
-    return fonts.length ?
-      FontList({
-        fonts,
-        fontOptions,
-        onPreviewTextChange,
-        onToggleFontHighlight,
-      })
-      :
-      dom.div(
-        {
-          className: "devtools-sidepanel-no-result",
-        },
-        getStr("fontinspector.noFontsUsedOnCurrentElement")
-      );
-  }
-
   renderFonts() {
     const {
       fontData,
       fontOptions,
       onPreviewTextChange,
     } = this.props;
 
-    const header = Services.prefs.getBoolPref(PREF_FONT_EDITOR)
-      ? getStr("fontinspector.allFontsOnPageHeader")
-      : getStr("fontinspector.otherFontsInPageHeader");
-
-    const fonts = Services.prefs.getBoolPref(PREF_FONT_EDITOR)
-      ? fontData.allFonts
-      : fontData.otherFonts;
+    const fonts = fontData.allFonts;
 
     if (!fonts.length) {
       return null;
@@ -81,7 +47,7 @@ class FontOverview extends PureComponent {
     return Accordion({
       items: [
         {
-          header,
+          header: getStr("fontinspector.allFontsOnPageHeader"),
           component: FontList,
           componentProps: {
             fontOptions,
@@ -100,8 +66,6 @@ class FontOverview extends PureComponent {
       {
         id: "font-container",
       },
-      // Render element fonts only when the Font Editor is not enabled.
-      !Services.prefs.getBoolPref(PREF_FONT_EDITOR) && this.renderElementFonts(),
       this.renderFonts()
     );
   }

diff --git a/devtools/client/inspector/fonts/components/FontsApp.js b/devtools/client/inspector/fonts/components/FontsApp.js
@@ -19,7 +19,6 @@ class FontsApp extends PureComponent {
     return {
       fontData: PropTypes.shape(Types.fontData).isRequired,
       fontEditor: PropTypes.shape(Types.fontEditor).isRequired,
-      fontEditorEnabled: PropTypes.bool.isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       onInstanceChange: PropTypes.func.isRequired,
       onPreviewTextChange: PropTypes.func.isRequired,
@@ -32,7 +31,6 @@ class FontsApp extends PureComponent {
     const {
       fontData,
       fontEditor,
-      fontEditorEnabled,
       fontOptions,
       onInstanceChange,
       onPreviewTextChange,
@@ -45,7 +43,7 @@ class FontsApp extends PureComponent {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-fontinspector",
       },
-      fontEditorEnabled && FontEditor({
+      FontEditor({
         fontEditor,
         onInstanceChange,
         onPropertyChange,

diff --git a/devtools/client/inspector/fonts/fonts.js b/devtools/client/inspector/fonts/fonts.js
@@ -6,7 +6,6 @@
 
 "use strict";
 
-const Services = require("Services");
 const { gDevTools } = require("devtools/client/framework/devtools");
 const { getColor } = require("devtools/client/shared/theme");
 const { createFactory, createElement } = require("devtools/client/shared/vendor/react");
@@ -45,7 +44,6 @@ const FONT_PROPERTIES = [
   "font-weight",
   "line-height",
 ];
-const PREF_FONT_EDITOR = "devtools.inspector.fonteditor.enabled";
 const REGISTERED_AXES_TO_FONT_PROPERTIES = {
   "ital": "font-style",
   "opsz": "font-optical-sizing",
@@ -99,7 +97,6 @@ class FontInspector {
     }
 
     const fontsApp = FontsApp({
-      fontEditorEnabled: Services.prefs.getBoolPref(PREF_FONT_EDITOR),
       onInstanceChange: this.onInstanceChange,
       onToggleFontHighlight: this.onToggleFontHighlight,
       onPreviewTextChange: this.onPreviewTextChange,
@@ -807,11 +804,6 @@ class FontInspector {
    * and the ones already in the store to decide if to update the font editor state.
    */
   async refreshFontEditor() {
-    // Early return if pref for font editor is not enabled.
-    if (!Services.prefs.getBoolPref(PREF_FONT_EDITOR)) {
-      return;
-    }
-
     if (!this.store || !this.isSelectedNodeValid()) {
       if (this.inspector.selection.isPseudoElementNode()) {
         const noPseudoWarning = getStr("fontinspector.noPseduoWarning");

diff --git a/devtools/client/inspector/fonts/test/browser.ini b/devtools/client/inspector/fonts/test/browser.ini
@@ -28,6 +28,7 @@ subsuite = clipboard
 [browser_fontinspector_input-element-used-font.js]
 [browser_fontinspector_no-fonts.js]
 [browser_fontinspector_other-fonts.js]
+skip-if = true # Note to reviewer: this test will be removed in next part of this series.
 [browser_fontinspector_reveal-in-page.js]
 [browser_fontinspector_text-node.js]
 [browser_fontinspector_theme-change.js]
diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector.js b/devtools/client/inspector/fonts/test/browser_fontinspector.js
@@ -8,7 +8,6 @@ requestLongerTimeout(2);
 const TEST_URI = URL_ROOT + "doc_browser_fontinspector.html";
 
 add_task(async function() {
-  await pushPref("devtools.inspector.fonteditor.enabled", true);
   const { inspector, view } = await openFontInspectorForURL(TEST_URI);
   ok(!!view, "Font inspector document is alive.");
 

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_all-fonts.js b/devtools/client/inspector/fonts/test/browser_fontinspector_all-fonts.js
@@ -9,7 +9,6 @@
 const TEST_URI = URL_ROOT + "doc_browser_fontinspector.html";
 
 add_task(async function() {
-  await pushPref("devtools.inspector.fonteditor.enabled", true);
   const { view } = await openFontInspectorForURL(TEST_URI);
   const viewDoc = view.document;
 

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_copy-URL.js b/devtools/client/inspector/fonts/test/browser_fontinspector_copy-URL.js
@@ -10,7 +10,6 @@
 const TEST_URI = URL_ROOT + "doc_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("div", inspector);

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_edit-previews.js b/devtools/client/inspector/fonts/test/browser_fontinspector_edit-previews.js
@@ -10,7 +10,6 @@
 const TEST_URI = URL_ROOT + "doc_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("div", inspector);

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js
@@ -12,7 +12,6 @@
 const TEST_URI = URL_ROOT + "doc_browser_fontinspector_iframe.html";
 
 add_task(async function() {
-  await pushPref("devtools.inspector.fonteditor.enabled", true);
   const { inspector, view } = await openFontInspectorForURL(TEST_URI);
   const viewDoc = view.document;
   const property = "font-size";

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_editor-keywords.js b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-keywords.js
@@ -11,7 +11,6 @@
 const TEST_URI = URL_ROOT + "doc_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;
 

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_editor-values.js b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-values.js
@@ -6,7 +6,6 @@
 const TEST_URI = URL_ROOT + "doc_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;
 

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_expand-css-code.js b/devtools/client/inspector/fonts/test/browser_fontinspector_expand-css-code.js
@@ -9,7 +9,6 @@
 const TEST_URI = URL_ROOT + "doc_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("div", inspector);

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_no-fonts.js b/devtools/client/inspector/fonts/test/browser_fontinspector_no-fonts.js
@@ -10,7 +10,6 @@
 const TEST_URI = URL_ROOT + "doc_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);

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js b/devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js
@@ -8,10 +8,6 @@
 const TEST_URI = URL_ROOT + "doc_browser_fontinspector.html";
 
 add_task(async function() {
-  await pushPref("devtools.inspector.fonteditor.enabled", true);
-  // Enable the feature, since it's off by default for now.
-  await pushPref("devtools.inspector.fonthighlighter.enabled", true);
-
   // Make sure the toolbox is tall enough to accomodate all fonts, otherwise mouseover
   // events simulation will fail.
   await pushPref("devtools.toolbox.footer.height", 500);

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_text-node.js b/devtools/client/inspector/fonts/test/browser_fontinspector_text-node.js
@@ -9,7 +9,6 @@
 const TEST_URI = URL_ROOT + "doc_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;
 

diff --git a/devtools/client/inspector/fonts/test/browser_fontinspector_theme-change.js b/devtools/client/inspector/fonts/test/browser_fontinspector_theme-change.js
@@ -18,7 +18,6 @@ registerCleanupFunction(() => {
 });
 
 add_task(async function() {
-  await pushPref("devtools.inspector.fonteditor.enabled", true);
   const { inspector, view } = await openFontInspectorForURL(TEST_URI);
   const viewDoc = view.document;
 

diff --git a/devtools/client/inspector/fonts/test/head.js b/devtools/client/inspector/fonts/test/head.js
@@ -28,13 +28,8 @@ selectNode = async function(node, inspector, reason) {
   const onEditorUpdated = inspector.once("fonteditor-updated");
   await _selectNode(node, inspector, reason);
 
-  if (Services.prefs.getBoolPref("devtools.inspector.fonteditor.enabled")) {
-    // Wait for both the font inspetor and font editor before proceeding.
-    await Promise.all([onInspectorUpdated, onEditorUpdated]);
-  } else {
-    // Wait just for the font inspector.
-    await onInspectorUpdated;
-  }
+  // Wait for both the font inspector and font editor before proceeding.
+  await Promise.all([onInspectorUpdated, onEditorUpdated]);
 };
 
 /**

diff --git a/devtools/client/locales/en-US/font-inspector.properties b/devtools/client/locales/en-US/font-inspector.properties
@@ -13,10 +13,6 @@ fontinspector.system=system
 # no fonts were used on the selected element.
 fontinspector.noFontsUsedOnCurrentElement=No fonts used on 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.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

diff --git a/devtools/client/preferences/devtools-client.js b/devtools/client/preferences/devtools-client.js
@@ -52,8 +52,6 @@ pref("devtools.inspector.showAllAnonymousContent", false);
 pref("devtools.inspector.showUserAgentShadowRoots", false);
 // Enable the CSS shapes highlighter
 pref("devtools.inspector.shapesHighlighter.enabled", true);
-// Enable the Font Editor
-pref("devtools.inspector.fonteditor.enabled", true);
 // Enable the font highlight-on-hover feature
 pref("devtools.inspector.fonthighlighter.enabled", true);
 // Enable tracking of style changes and the Changes panel in the Inspector