Bug 1523305 - Show Font Editor in read-only mode for pseudo-elements. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 04 Feb 2019 12:05:23 +0000
changeset 456610 6059b24342142db033db72eeae6cc39bad7e2b52
parent 456609 3a1f671e95267c76bf6f95915d816ee58a85a836
child 456611 4fcae0e31524da7ca7477e076738f9cf826939bd
push id35496
push userbtara@mozilla.com
push dateMon, 04 Feb 2019 17:36:40 +0000
treeherdermozilla-central@be1beccfb86d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1523305
milestone67.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 1523305 - Show Font Editor in read-only mode for pseudo-elements. r=gl Adds a new `disabled` property to the Font Editor Redux store applicable to all input fields. When inspecting a pseudo-element, this `disabled` property is set to true. This allows the pseudo-element to be inspected, but prevents editing font property values because it's currently not possible to write them back to CSS rules other than element inline styles. Differential Revision: https://phabricator.services.mozilla.com/D18364
devtools/client/inspector/fonts/actions/font-editor.js
devtools/client/inspector/fonts/actions/index.js
devtools/client/inspector/fonts/components/FontAxis.js
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/components/FontPropertyValue.js
devtools/client/inspector/fonts/components/FontSize.js
devtools/client/inspector/fonts/components/FontStyle.js
devtools/client/inspector/fonts/components/FontWeight.js
devtools/client/inspector/fonts/components/LineHeight.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/reducers/font-editor.js
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/common.css
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/actions/font-editor.js
+++ b/devtools/client/inspector/fonts/actions/font-editor.js
@@ -2,31 +2,39 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {
   APPLY_FONT_VARIATION_INSTANCE,
   RESET_EDITOR,
+  SET_FONT_EDITOR_DISABLED,
   UPDATE_AXIS_VALUE,
   UPDATE_CUSTOM_INSTANCE,
   UPDATE_EDITOR_STATE,
   UPDATE_PROPERTY_VALUE,
   UPDATE_WARNING_MESSAGE,
 } = require("./index");
 
 module.exports = {
 
   resetFontEditor() {
     return {
       type: RESET_EDITOR,
     };
   },
 
+  setEditorDisabled(disabled = false) {
+    return {
+      type: SET_FONT_EDITOR_DISABLED,
+      disabled,
+    };
+  },
+
   applyInstance(name, values) {
     return {
       type: APPLY_FONT_VARIATION_INSTANCE,
       name,
       values,
     };
   },
 
--- a/devtools/client/inspector/fonts/actions/index.js
+++ b/devtools/client/inspector/fonts/actions/index.js
@@ -2,35 +2,34 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { createEnum } = require("devtools/client/shared/enum");
 
 createEnum([
+  // Reset font editor to intial state.
+  "RESET_EDITOR",
+
+  // Set the font editor disabled state which prevents users from interacting with inputs.
+  "SET_FONT_EDITOR_DISABLED",
+
+  // Apply the variation settings of a font instance.
+  "APPLY_FONT_VARIATION_INSTANCE",
 
   // Update the custom font variation instance with the current axes values.
   "UPDATE_CUSTOM_INSTANCE",
 
-  // Reset font editor to intial state.
-  "RESET_EDITOR",
-
-  // Apply the variation settings of a font instance.
-  "APPLY_FONT_VARIATION_INSTANCE",
-
   // Update the value of a variable font axis.
   "UPDATE_AXIS_VALUE",
 
   // Update font editor with applicable fonts and user-defined CSS font properties.
   "UPDATE_EDITOR_STATE",
 
-  // Toggle the visibiltiy of the font editor
-  "UPDATE_EDITOR_VISIBILITY",
-
   // Update the list of fonts.
   "UPDATE_FONTS",
 
   // Update the preview text.
   "UPDATE_PREVIEW_TEXT",
 
   // Update the value of a CSS font property
   "UPDATE_PROPERTY_VALUE",
--- a/devtools/client/inspector/fonts/components/FontAxis.js
+++ b/devtools/client/inspector/fonts/components/FontAxis.js
@@ -10,16 +10,17 @@ const PropTypes = require("devtools/clie
 const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 
 const Types = require("../types");
 
 class FontAxis extends PureComponent {
   static get propTypes() {
     return {
       axis: PropTypes.shape(Types.fontVariationAxis),
+      disabled: PropTypes.bool.isRequired,
       onChange: PropTypes.func.isRequired,
       value: PropTypes.string.isRequired,
     };
   }
 
   /**
    * Naive implementation to get increment step for variable font axis that ensures
    * fine grained control based on range of values between min and max.
@@ -46,16 +47,17 @@ class FontAxis extends PureComponent {
     return step;
   }
 
   render() {
     const { axis, value, onChange } = this.props;
 
     return FontPropertyValue({
       className: "font-control-axis",
+      disabled: this.props.disabled,
       label: axis.name,
       min: axis.minValue,
       minLabel: true,
       max: axis.maxValue,
       maxLabel: true,
       name: axis.tag,
       nameLabel: true,
       onChange,
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -49,16 +49,17 @@ class FontEditor extends PureComponent {
     if (!fontAxes.length) {
       return null;
     }
 
     return fontAxes.map(axis => {
       return FontAxis({
         key: axis.tag,
         axis,
+        disabled: this.props.fontEditor.disabled,
         onChange: this.props.onPropertyChange,
         minLabel: true,
         maxLabel: true,
         value: editedAxes[axis.tag] || axis.defaultValue,
       });
     });
   }
 
@@ -138,39 +139,43 @@ class FontEditor extends PureComponent {
         family),
       group
     );
   }
 
   renderFontSize(value) {
     return value !== null && FontSize({
       key: `${this.props.fontEditor.id}:font-size`,
+      disabled: this.props.fontEditor.disabled,
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   renderLineHeight(value) {
     return value !== null && LineHeight({
       key: `${this.props.fontEditor.id}:line-height`,
+      disabled: this.props.fontEditor.disabled,
       onChange: this.props.onPropertyChange,
       value,
     });
   }
 
   renderFontStyle(value) {
     return value && FontStyle({
       onChange: this.props.onPropertyChange,
+      disabled: this.props.fontEditor.disabled,
       value,
     });
   }
 
   renderFontWeight(value) {
     return value !== null && FontWeight({
       onChange: this.props.onPropertyChange,
+      disabled: this.props.fontEditor.disabled,
       value,
     });
   }
 
   /**
    * Get a dropdown which allows selecting between variation instances defined by a font.
    *
    * @param {Array} fontInstances
--- a/devtools/client/inspector/fonts/components/FontPropertyValue.js
+++ b/devtools/client/inspector/fonts/components/FontPropertyValue.js
@@ -19,16 +19,17 @@ const { toFixed } = require("../utils/fo
 const AUTOINCREMENT_DELAY = 1000;
 
 class FontPropertyValue extends PureComponent {
   static get propTypes() {
     return {
       autoIncrement: PropTypes.bool,
       className: PropTypes.string,
       defaultValue: PropTypes.number,
+      disabled: PropTypes.bool.isRequired,
       label: PropTypes.string.isRequired,
       min: PropTypes.number.isRequired,
       // Whether to show the `min` prop value as a label.
       minLabel: PropTypes.bool,
       max: PropTypes.number.isRequired,
       // Whether to show the `max` prop value as a label.
       maxLabel: PropTypes.bool,
       name: PropTypes.string.isRequired,
@@ -370,22 +371,23 @@ class FontPropertyValue extends PureComp
     const options = this.props.unitOptions.includes(this.props.unit) ?
       this.props.unitOptions
       :
       this.props.unitOptions.concat([this.props.unit]);
 
     return dom.select(
       {
         className: "font-value-select",
+        disabled: this.props.disabled,
         onChange: this.onUnitChange,
+        value: this.props.unit,
       },
       options.map(unit => {
         return dom.option(
           {
-            selected: unit === this.props.unit,
             value: unit,
           },
           unit
         );
       })
     );
   }
 
@@ -445,36 +447,39 @@ class FontPropertyValue extends PureComp
       {
         ...defaults,
         onKeyDown: this.onKeyDown,
         onKeyUp: this.onKeyUp,
         onMouseDown: this.onMouseDown,
         onMouseMove: this.onMouseMove,
         onMouseUp: this.onMouseUp,
         className: "font-value-slider",
+        disabled: this.props.disabled,
         name: this.props.name,
         title: this.props.label,
         type: "range",
       }
     );
 
     const input = dom.input(
       {
         ...defaults,
         // Remove upper limit from number input if it is allowed to auto-increment.
         max: this.props.autoIncrement ? null : this.props.max,
         name: this.props.name,
         className: "font-value-input",
+        disabled: this.props.disabled,
         type: "number",
       }
     );
 
     return dom.label(
       {
         className: `font-control ${this.props.className}`,
+        disabled: this.props.disabled,
       },
       dom.div(
         {
           className: "font-control-label",
           title: this.props.label,
         },
         this.renderLabelContent()
       ),
--- a/devtools/client/inspector/fonts/components/FontSize.js
+++ b/devtools/client/inspector/fonts/components/FontSize.js
@@ -10,16 +10,17 @@ const PropTypes = require("devtools/clie
 const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 
 const { getStr } = require("../utils/l10n");
 const { getUnitFromValue, getStepForUnit } = require("../utils/font-utils");
 
 class FontSize extends PureComponent {
   static get propTypes() {
     return {
+      disabled: PropTypes.bool.isRequired,
       onChange: PropTypes.func.isRequired,
       value: PropTypes.string.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
     this.historicMax = {};
@@ -54,16 +55,17 @@ class FontSize extends PureComponent {
     // max value of the input and slider. Without this memoization, the value and slider
     // thumb get clamped at the upper bound while decrementing an out-of-bounds value.
     this.historicMax[unit] = this.historicMax[unit]
       ? Math.max(this.historicMax[unit], max)
       : max;
 
     return FontPropertyValue({
       autoIncrement: true,
+      disabled: this.props.disabled,
       label: getStr("fontinspector.fontSizeLabel"),
       min: 0,
       max: this.historicMax[unit],
       name: "font-size",
       onChange: this.props.onChange,
       step: getStepForUnit(unit),
       unit,
       unitOptions: ["em", "rem", "%", "px", "vh", "vw"],
--- a/devtools/client/inspector/fonts/components/FontStyle.js
+++ b/devtools/client/inspector/fonts/components/FontStyle.js
@@ -8,16 +8,17 @@ const { PureComponent } = require("devto
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const { getStr } = require("../utils/l10n");
 
 class FontStyle extends PureComponent {
   static get propTypes() {
     return {
+      disabled: PropTypes.bool.isRequired,
       onChange: PropTypes.func.isRequired,
       value: PropTypes.string.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
     this.name = "font-style";
@@ -42,16 +43,17 @@ class FontStyle extends PureComponent {
       dom.div(
         {
           className: "font-control-input",
         },
         dom.input(
           {
             checked: this.props.value === "italic" || this.props.value === "oblique",
             className: "devtools-checkbox-toggle",
+            disabled: this.props.disabled,
             name: this.name,
             onChange: this.onToggle,
             type: "checkbox",
           }
         )
       )
     );
   }
--- a/devtools/client/inspector/fonts/components/FontWeight.js
+++ b/devtools/client/inspector/fonts/components/FontWeight.js
@@ -9,23 +9,25 @@ const PropTypes = require("devtools/clie
 
 const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 
 const { getStr } = require("../utils/l10n");
 
 class FontWeight extends PureComponent {
   static get propTypes() {
     return {
+      disabled: PropTypes.bool.isRequired,
       onChange: PropTypes.func.isRequired,
       value: PropTypes.string.isRequired,
     };
   }
 
   render() {
     return FontPropertyValue({
+      disabled: this.props.disabled,
       label: getStr("fontinspector.fontWeightLabel"),
       min: 100,
       max: 900,
       name: "font-weight",
       onChange: this.props.onChange,
       step: 100,
       unit: null,
       value: parseFloat(this.props.value),
--- a/devtools/client/inspector/fonts/components/LineHeight.js
+++ b/devtools/client/inspector/fonts/components/LineHeight.js
@@ -10,16 +10,17 @@ const PropTypes = require("devtools/clie
 const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 
 const { getStr } = require("../utils/l10n");
 const { getUnitFromValue, getStepForUnit } = require("../utils/font-utils");
 
 class LineHeight extends PureComponent {
   static get propTypes() {
     return {
+      disabled: PropTypes.bool.isRequired,
       onChange: PropTypes.func.isRequired,
       value: PropTypes.string.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
     this.historicMax = {};
@@ -57,16 +58,17 @@ class LineHeight extends PureComponent {
     // max value of the input and slider. Without this memoization, the value and slider
     // thumb get clamped at the upper bound while decrementing an out-of-bounds value.
     this.historicMax[unit] = this.historicMax[unit]
       ? Math.max(this.historicMax[unit], max)
       : max;
 
     return FontPropertyValue({
       autoIncrement: true,
+      disabled: this.props.disabled,
       label: getStr("fontinspector.lineHeightLabelCapitalized"),
       min: 0,
       max: this.historicMax[unit],
       name: "line-height",
       onChange: this.props.onChange,
       step: getStepForUnit(unit),
       unit,
       unitOptions: ["", "em", "%", "px"],
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -20,21 +20,21 @@ const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
 
 const { getStr } = require("./utils/l10n");
 const { parseFontVariationAxes } = require("./utils/font-utils");
 const { updateFonts } = require("./actions/fonts");
 const {
   applyInstance,
   resetFontEditor,
+  setEditorDisabled,
   updateAxis,
   updateCustomInstance,
   updateFontEditor,
   updateFontProperty,
-  updateWarningMessage,
 } = require("./actions/font-editor");
 const { updatePreviewText } = require("./actions/font-options");
 
 const CUSTOM_INSTANCE_NAME = getStr("fontinspector.customInstanceName");
 const FONT_PROPERTIES = [
   "font-family",
   "font-optical-sizing",
   "font-size",
@@ -544,18 +544,17 @@ class FontInspector {
    * Check if a selected node exists and fonts can apply to it.
    *
    * @return {Boolean}
    */
   isSelectedNodeValid() {
     return this.inspector &&
            this.inspector.selection.nodeFront &&
            this.inspector.selection.isConnected() &&
-           this.inspector.selection.isElementNode() &&
-           !this.inspector.selection.isPseudoElementNode();
+           this.inspector.selection.isElementNode();
   }
 
   /**
    * Upon a new node selection, log some interesting telemetry probes.
    */
   logTelemetryProbesOnNewNode() {
     const { fontEditor } = this.store.getState();
     const { telemetry } = this.inspector;
@@ -789,23 +788,16 @@ class FontInspector {
    * - during initial setup;
    * - when a new node is selected;
    * - when any property is changed in the Rule view.
    * For the latter case, we compare between the latest computed style font properties
    * and the ones already in the store to decide if to update the font editor state.
    */
   async refreshFontEditor() {
     if (!this.store || !this.isSelectedNodeValid()) {
-      if (this.inspector.selection.isPseudoElementNode()) {
-        const noPseudoWarning = getStr("fontinspector.noPseduoWarning");
-        this.store.dispatch(resetFontEditor());
-        this.store.dispatch(updateWarningMessage(noPseudoWarning));
-        return;
-      }
-
       // If the selection is a TextNode, switch selection to be its parent node.
       if (this.inspector.selection.isTextNode()) {
         const selection = this.inspector.selection;
         selection.setNodeFront(selection.nodeFront.parentNode());
         return;
       }
 
       this.store.dispatch(resetFontEditor());
@@ -860,16 +852,19 @@ class FontInspector {
     const properties = await this.getFontProperties();
     // Assign writer methods to each axis defined in font-variation-settings.
     const axes = parseFontVariationAxes(properties["font-variation-settings"]);
     Object.keys(axes).map(axis => {
       this.writers.set(axis, this.getWriterForAxis(axis));
     });
 
     this.store.dispatch(updateFontEditor(fonts, properties, node.actorID));
+    const isPseudo = this.inspector.selection.isPseudoElementNode();
+    this.store.dispatch(setEditorDisabled(isPseudo));
+
     this.inspector.emit("fonteditor-updated");
     // Listen to manual changes in the Rule view that could update the Font Editor state
     this.ruleView.on("property-value-updated", this.onRulePropertyUpdated);
   }
 
   /**
    * Capture the state of all variation axes. Allows the user to return to this state with
    * the "Custom" instance after they've selected a font-defined named variation instance.
--- a/devtools/client/inspector/fonts/reducers/font-editor.js
+++ b/devtools/client/inspector/fonts/reducers/font-editor.js
@@ -5,28 +5,31 @@
 "use strict";
 
 const { getStr } = require("../utils/l10n");
 const { parseFontVariationAxes } = require("../utils/font-utils");
 
 const {
   APPLY_FONT_VARIATION_INSTANCE,
   RESET_EDITOR,
+  SET_FONT_EDITOR_DISABLED,
   UPDATE_AXIS_VALUE,
   UPDATE_CUSTOM_INSTANCE,
   UPDATE_EDITOR_STATE,
   UPDATE_PROPERTY_VALUE,
   UPDATE_WARNING_MESSAGE,
 } = require("../actions/index");
 
 const INITIAL_STATE = {
   // Variable font axes.
   axes: {},
   // Copy of the most recent axes values. Used to revert from a named instance.
   customInstanceValues: [],
+  // When true, prevent users from interacting with inputs in the font editor.
+  disabled: false,
   // Fonts used on the selected element.
   fonts: [],
   // Current selected font variation instance.
   instance: {
     name: getStr("fontinspector.customInstanceName"),
     values: [],
   },
   // CSS font properties defined on the selected rule.
@@ -70,16 +73,20 @@ const reducers = {
   [UPDATE_CUSTOM_INSTANCE](state) {
     const newState = { ...state };
     newState.customInstanceValues = Object.keys(state.axes).map(axis => {
       return { axis: [axis], value: state.axes[axis] };
     });
     return newState;
   },
 
+  [SET_FONT_EDITOR_DISABLED](state, { disabled }) {
+    return { ...state, disabled };
+  },
+
   [UPDATE_EDITOR_STATE](state, { fonts, properties, id }) {
     const axes = parseFontVariationAxes(properties["font-variation-settings"]);
 
     // If not defined in font-variation-settings, setup "wght" axis with the value of
     // "font-weight" if it is numeric and not a keyword.
     const weight = properties["font-weight"];
     if (axes.wght === undefined && parseFloat(weight).toString() === weight.toString()) {
       axes.wght = weight;
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -44,20 +44,16 @@ fontinspector.fontWeightLabel=Weight
 fontinspector.fontItalicLabel=Italic
 
 # LOCALIZATION NOTE (fontinspector.showMore): Label for a collapsed list of fonts.
 fontinspector.showMore=Show more
 
 # LOCALIZATION NOTE (fontinspector.showLess): Label for an expanded list of fonts.
 fontinspector.showLess=Show less
 
-# LOCALIZATION NOTE (fontinspector.noPseduoWarning): Warning message shown when the
-# selected element is a pseudo-element that the font editor cannot work with.
-fontinspector.noPseduoWarning=Pseudo-elements are not currently supported.
-
 # LOCALIZATION NOTE (fontinspector.lineHeightLabelCapitalized): Label for the UI to change the line height in the font editor.
 fontinspector.lineHeightLabelCapitalized=Line Height
 
 # LOCALIZATION NOTE (fontinspector.allFontsOnPageHeader): Header for the section listing
 # all the fonts on the current page.
 fontinspector.allFontsOnPageHeader=All fonts on page
 
 # LOCALIZATION NOTE (fontinspector.fontsUsedLabel): Label for the Font Editor section
--- a/devtools/client/themes/common.css
+++ b/devtools/client/themes/common.css
@@ -821,16 +821,20 @@ checkbox:-moz-focusring {
   height: 1em;
   border-radius: 1em;
   border: 1px solid transparent;
   box-sizing: content-box;
   /* Animate the thumb position between states of the checkbox. */
   transition: background-color .1s ease-out;
 }
 
+.devtools-checkbox-toggle[disabled]{
+  opacity: 0.5;
+}
+
 /* For right-to-left layout, the toggle thumb goes in the opposite direction. */
 html[dir="rtl"] .devtools-checkbox-toggle {
   --x-pos: -.15em;
 }
 
 .devtools-checkbox-toggle:not(:checked):focus {
   border-color: var(--blue-55);
 }
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -249,16 +249,23 @@
 
 .font-value-input,
 .font-value-select {
   color: var(--input-text-color);
   border: 1px solid var(--input-border-color);
   background-color: var(--input-background-color);
 }
 
+/* Styles for disabled input fields */
+.font-value-input[disabled],
+.font-value-select[disabled],
+.font-value-slider[disabled] {
+  opacity: 0.5;
+}
+
 /* Do not use browser "invalid" state */
 .font-value-slider:-moz-ui-invalid,
 .font-value-input:-moz-ui-invalid {
   box-shadow: none;
 }
 
 /* Do not show dotted line focus outline */
 .font-value-input:-moz-focusring {