From c35f2a5608be11dde52a178541b0e7b1f29f0fa7 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Tue, 28 Mar 2023 15:37:07 +0000 Subject: [PATCH] Bug 1817639 - [devtools] Dynamically apply changes to the simplified highlighters preference r=nchevobbe,devtools-reviewers Depends on D170294 Differential Revision: https://phabricator.services.mozilla.com/D170291 --- devtools/client/framework/toolbox.js | 22 ++++++++++++ ...er_inspector_highlighter-reduced-motion.js | 24 ++++++++++++- devtools/server/actors/highlighters.css | 20 ++++++----- devtools/server/actors/highlighters.js | 11 +++++- .../server/actors/highlighters/accessible.js | 10 +++++- .../actors/highlighters/auto-refresh.js | 34 +++++++++++++++++++ .../server/actors/highlighters/box-model.js | 14 +++++++- .../server/actors/target-configuration.js | 2 ++ .../server/actors/targets/window-global.js | 6 ++++ 9 files changed, 130 insertions(+), 13 deletions(-) diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 2dad94e38c85..8c85fb907d13 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -290,6 +290,9 @@ function Toolbox(commands, selectedTool, hostType, contentWindow, frameId) { this._applyServiceWorkersTestingSettings = this._applyServiceWorkersTestingSettings.bind( this ); + this._applySimpleHighlightersSettings = this._applySimpleHighlightersSettings.bind( + this + ); this._saveSplitConsoleHeight = this._saveSplitConsoleHeight.bind(this); this._onFocus = this._onFocus.bind(this); this._onBlur = this._onBlur.bind(this); @@ -934,6 +937,10 @@ Toolbox.prototype = { "devtools.serviceWorkers.testing.enabled", this._applyServiceWorkersTestingSettings ); + Services.prefs.addObserver( + "devtools.inspector.simple-highlighters-reduced-motion", + this._applySimpleHighlightersSettings + ); Services.prefs.addObserver( BROWSERTOOLBOX_SCOPE_PREF, this._refreshHostTitle @@ -954,6 +961,7 @@ Toolbox.prototype = { // Forward configuration flags to the DevTools server. this._applyCacheSettings(); this._applyServiceWorkersTestingSettings(); + this._applySimpleHighlightersSettings(); this._addWindowListeners(); this._addChromeEventHandlerEvents(); @@ -2200,6 +2208,16 @@ Toolbox.prototype = { }); }, + /** + * Apply the current simple highlighters setting to this toolbox's tab. + */ + _applySimpleHighlightersSettings() { + const pref = "devtools.inspector.simple-highlighters-reduced-motion"; + this.commands.targetConfigurationCommand.updateConfiguration({ + useSimpleHighlightersForReducedMotion: Services.prefs.getBoolPref(pref), + }); + }, + /** * Update the visibility of the buttons. */ @@ -3990,6 +4008,10 @@ Toolbox.prototype = { "devtools.serviceWorkers.testing.enabled", this._applyServiceWorkersTestingSettings ); + Services.prefs.removeObserver( + "devtools.inspector.simple-highlighters-reduced-motion", + this._applySimpleHighlightersSettings + ); Services.prefs.removeObserver( BROWSERTOOLBOX_SCOPE_PREF, this._refreshHostTitle diff --git a/devtools/client/inspector/test/browser_inspector_highlighter-reduced-motion.js b/devtools/client/inspector/test/browser_inspector_highlighter-reduced-motion.js index e224c82969e6..19ae72b00551 100644 --- a/devtools/client/inspector/test/browser_inspector_highlighter-reduced-motion.js +++ b/devtools/client/inspector/test/browser_inspector_highlighter-reduced-motion.js @@ -7,7 +7,8 @@ // Check that the boxmodel highlighter is styled differently when // ui.prefersReducedMotion is enabled. -const TEST_URL = "data:text/html;charset=utf-8,

test1

test2

"; +const TEST_URL = + "data:text/html;charset=utf-8,

test1

test2

test3

"; add_task(async function() { info("Disable ui.prefersReducedMotion"); @@ -64,4 +65,25 @@ add_task(async function() { is(fill, "none", "If prefersReducedMotion is enabled, fill style is none"); await inspector.highlighters.hideHighlighterType(HIGHLIGHTER_TYPE); + + info("Disable simple highlighters"); + await pushPref( + "devtools.inspector.simple-highlighters-reduced-motion", + false + ); + + info("Highlight a node and check the highlighter is filled again"); + await selectAndHighlightNode("h3", inspector); + stroke = await getElementComputedStyle("box-model-content", "stroke"); + fill = await getElementComputedStyle("box-model-content", "fill"); + is( + stroke, + "none", + "If simple highlighters are disabled, stroke style is none" + ); + ok( + InspectorUtils.isValidCSSColor(fill), + "If simple highlighters are disabled, fill style is a valid color" + ); + await inspector.highlighters.hideHighlighterType(HIGHLIGHTER_TYPE); }); diff --git a/devtools/server/actors/highlighters.css b/devtools/server/actors/highlighters.css index ee609a3938c1..7abc77e5f233 100644 --- a/devtools/server/actors/highlighters.css +++ b/devtools/server/actors/highlighters.css @@ -128,27 +128,29 @@ } @media (prefers-reduced-motion) { - :-moz-native-anonymous .box-model-content, - :-moz-native-anonymous .box-model-padding, - :-moz-native-anonymous .box-model-border, - :-moz-native-anonymous .box-model-margin { + :-moz-native-anonymous .use-simple-highlighters :is( + .box-model-content, + .box-model-padding, + .box-model-border, + .box-model-margin + ) { fill: none; stroke-width: 3; } - :-moz-native-anonymous .box-model-content { + :-moz-native-anonymous .use-simple-highlighters .box-model-content { stroke: var(--highlighter-box-content-color); } - :-moz-native-anonymous .box-model-padding { + :-moz-native-anonymous .use-simple-highlighters .box-model-padding { stroke: var(--highlighter-box-padding-color); } - :-moz-native-anonymous .box-model-border { + :-moz-native-anonymous .use-simple-highlighters .box-model-border { stroke: var(--highlighter-box-border-color); } - :-moz-native-anonymous .box-model-margin { + :-moz-native-anonymous .use-simple-highlighters .box-model-margin { stroke: var(--highlighter-box-margin-color); } } @@ -945,7 +947,7 @@ } @media (prefers-reduced-motion) { - :-moz-native-anonymous .accessible-bounds { + :-moz-native-anonymous .use-simple-highlighters .accessible-bounds { fill: none; stroke: var(--highlighter-accessibility-bounds-color); stroke-width: 3; diff --git a/devtools/server/actors/highlighters.js b/devtools/server/actors/highlighters.js index 2c6a7527a8f5..7f99e9d5ea59 100644 --- a/devtools/server/actors/highlighters.js +++ b/devtools/server/actors/highlighters.js @@ -180,7 +180,12 @@ class HighlighterEnvironment extends EventEmitter { initFromTargetActor(targetActor) { this._targetActor = targetActor; - const relayedEvents = ["window-ready", "navigate", "will-navigate"]; + const relayedEvents = [ + "window-ready", + "navigate", + "will-navigate", + "use-simple-highlighters-updated", + ]; this._abortController = new AbortController(); const signal = this._abortController.signal; @@ -245,6 +250,10 @@ class HighlighterEnvironment extends EventEmitter { return isXUL(this.window); } + get useSimpleHighlightersForReducedMotion() { + return this._targetActor?._useSimpleHighlightersForReducedMotion; + } + get window() { if (!this.isInitialized) { throw new Error( diff --git a/devtools/server/actors/highlighters/accessible.js b/devtools/server/actors/highlighters/accessible.js index 699c903938fe..71124239f29e 100644 --- a/devtools/server/actors/highlighters/accessible.js +++ b/devtools/server/actors/highlighters/accessible.js @@ -98,6 +98,10 @@ class AccessibleHighlighter extends AutoRefreshHighlighter { return true; } + get supportsSimpleHighlighters() { + return true; + } + /** * Build highlighter markup. * @@ -115,7 +119,11 @@ class AccessibleHighlighter extends AutoRefreshHighlighter { parent: container, attributes: { id: "root", - class: "root", + class: + "root" + + (this.highlighterEnv.useSimpleHighlightersForReducedMotion + ? " use-simple-highlighters" + : ""), }, prefix: this.ID_CLASS_PREFIX, }); diff --git a/devtools/server/actors/highlighters/auto-refresh.js b/devtools/server/actors/highlighters/auto-refresh.js index 9e10ca55171e..652e8aa817d9 100644 --- a/devtools/server/actors/highlighters/auto-refresh.js +++ b/devtools/server/actors/highlighters/auto-refresh.js @@ -75,6 +75,12 @@ class AutoRefreshHighlighter extends EventEmitter { this.highlighterEnv = highlighterEnv; + this._updateSimpleHighlighters = this._updateSimpleHighlighters.bind(this); + this.highlighterEnv.on( + "use-simple-highlighters-updated", + this._updateSimpleHighlighters + ); + this.currentNode = null; this.currentQuads = {}; @@ -102,6 +108,10 @@ class AutoRefreshHighlighter extends EventEmitter { return this.win; } + get supportsSimpleHighlighters() { + return false; + } + /** * Show the highlighter on a given node * @param {DOMNode} node @@ -307,9 +317,33 @@ class AutoRefreshHighlighter extends EventEmitter { this.rafID = this.rafWin = null; } + _updateSimpleHighlighters() { + if (!this.supportsSimpleHighlighters) { + return; + } + + const root = this.getElement("root"); + if (!root) { + // Highlighters which support simple highlighters are expected to use a + // root element with the id "root". + return; + } + + // Add/remove the `user-simple-highlighters` class based on the current + // toolbox configuration. + root.classList.toggle( + "use-simple-highlighters", + this.highlighterEnv.useSimpleHighlightersForReducedMotion + ); + } + destroy() { this.hide(); + this.highlighterEnv.off( + "use-simple-highlighters-updated", + this._updateSimpleHighlighters + ); this.highlighterEnv = null; this.currentNode = null; } diff --git a/devtools/server/actors/highlighters/box-model.js b/devtools/server/actors/highlighters/box-model.js index e39c98d9637c..13dd2caa1f81 100644 --- a/devtools/server/actors/highlighters/box-model.js +++ b/devtools/server/actors/highlighters/box-model.js @@ -126,11 +126,17 @@ class BoxModelHighlighter extends AutoRefreshHighlighter { return true; } + get supportsSimpleHighlighters() { + return true; + } + _buildMarkup() { const highlighterContainer = this.markup.anonymousContentDocument.createElement( "div" ); highlighterContainer.className = "highlighter-container box-model"; + + this.highlighterContainer = highlighterContainer; // We need a better solution for how to handle the highlighter from the // accessibility standpoint. For now, in order to avoid displaying it in the // accessibility tree lets hide it altogether. See bug 1598667 for more @@ -142,7 +148,11 @@ class BoxModelHighlighter extends AutoRefreshHighlighter { parent: highlighterContainer, attributes: { id: "root", - class: "root", + class: + "root" + + (this.highlighterEnv.useSimpleHighlightersForReducedMotion + ? " use-simple-highlighters" + : ""), role: "presentation", }, prefix: this.ID_CLASS_PREFIX, @@ -383,7 +393,9 @@ class BoxModelHighlighter extends AutoRefreshHighlighter { } else { this._hideInfobar(); } + this._updateSimpleHighlighters(); this._showBoxModel(); + shown = true; } else { // Nothing to highlight (0px rectangle like a