From d5f2881bd2a6d91cb489a548f4fdbd1b80b44899 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Tue, 13 Nov 2018 18:55:30 +0000 Subject: [PATCH] Bug 1485324 - (Part 1) Remove pref for Font Editor and obsolete conditionals; 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 --- .../inspector/fonts/components/FontList.js | 17 +++----- .../fonts/components/FontOverview.js | 40 +------------------ .../inspector/fonts/components/FontsApp.js | 4 +- devtools/client/inspector/fonts/fonts.js | 8 ---- .../client/inspector/fonts/test/browser.ini | 1 + .../fonts/test/browser_fontinspector.js | 1 - .../test/browser_fontinspector_all-fonts.js | 1 - .../test/browser_fontinspector_copy-URL.js | 1 - .../browser_fontinspector_edit-previews.js | 1 - ...ntinspector_editor-font-size-conversion.js | 1 - .../browser_fontinspector_editor-keywords.js | 1 - .../browser_fontinspector_editor-values.js | 1 - .../browser_fontinspector_expand-css-code.js | 1 - .../test/browser_fontinspector_no-fonts.js | 1 - .../browser_fontinspector_reveal-in-page.js | 4 -- .../test/browser_fontinspector_text-node.js | 1 - .../browser_fontinspector_theme-change.js | 1 - devtools/client/inspector/fonts/test/head.js | 9 +---- .../locales/en-US/font-inspector.properties | 4 -- .../client/preferences/devtools-client.js | 2 - 20 files changed, 11 insertions(+), 89 deletions(-) diff --git a/devtools/client/inspector/fonts/components/FontList.js b/devtools/client/inspector/fonts/components/FontList.js index 1f5420c3ec87..03bdec66d02a 100644 --- 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 index 89b172dfee69..fd0146bddcf5 100644 --- a/devtools/client/inspector/fonts/components/FontOverview.js +++ b/devtools/client/inspector/fonts/components/FontOverview.js @@ -7,7 +7,6 @@ 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")); @@ -15,8 +14,6 @@ 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,31 +31,6 @@ 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, @@ -66,13 +38,7 @@ class FontOverview extends PureComponent { 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 index 4463504c7263..7127b03c7309 100644 --- 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 index 8c1b06581512..383b46fbf8dc 100644 --- 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 index 271eb3e0baa3..e6b4900de058 100644 --- 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 index 71251929c11d..e19f94aad6a3 100644 --- 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 index 229388c1a36a..66fadcad107a 100644 --- 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 index 75053da9bc9c..ce02c2c59226 100644 --- 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 index f4de10e44a81..641322059d9a 100644 --- 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 index 29f754a40fea..62bf3ff3b52d 100644 --- 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 index f622d09d1fe3..0c4fa1fc716a 100644 --- 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 index c91069c466f7..25a17514ce78 100644 --- 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 index 964f2cabb906..958e0fac648a 100644 --- 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 index 879329d5b3c0..ddc7212fd879 100644 --- 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 index 911b61f17381..8c19a7134778 100644 --- 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 index 8a1706ad6877..9fd5909b9ac4 100644 --- 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 index fee3c3d8fcd8..62c11e7134c3 100644 --- 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 index 99675e4793ef..ac8eecabb9a5 100644 --- 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 index 3516ed252a19..dd8107ccc313 100644 --- 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 index be516c311acf..61eb9acc33b5 100644 --- 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