diff --git a/devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js b/devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js index 01aa026ea6dc..60128cd18fa8 100644 --- a/devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js +++ b/devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js @@ -36,6 +36,11 @@ add_task(async function() { const { gridInspector, inspector } = await openLayoutView(); const { document: doc } = gridInspector; const { highlighters, store } = inspector; + const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.GRID; + const { + waitForHighlighterTypeRestored, + waitForHighlighterTypeDiscarded, + } = getHighlighterTestHelpers(inspector); await selectNode("#grid", inspector); const gridList = doc.getElementById("grid-list"); @@ -65,7 +70,7 @@ add_task(async function() { "Reload the page, expect the highlighter to be displayed once again and " + "grid is checked" ); - let onStateRestored = highlighters.once("grid-state-restored"); + const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE); let onGridListRestored = waitUntilState( store, state => state.grids.length == 1 && state.grids[0].highlighted @@ -76,13 +81,12 @@ add_task(async function() { info("Wait for inspector to be reloaded after page reload"); await onReloaded; - let { restored } = await onStateRestored; + await onRestored; await onGridListRestored; info( "Check that the grid highlighter can be displayed after reloading the page" ); - ok(restored, "The highlighter state was restored"); is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown."); is( highlighters.state.grids.size, @@ -96,19 +100,18 @@ add_task(async function() { ); const otherUri = "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI); - onStateRestored = highlighters.once("grid-state-restored"); + const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE); onGridListRestored = waitUntilState( store, state => state.grids.length == 1 && !state.grids[0].highlighted ); await navigateTo(otherUri); - ({ restored } = await onStateRestored); + await onDiscarded; await onGridListRestored; info( "Check that the grid highlighter is hidden after navigating to a different page" ); - ok(!restored, "The highlighter state was not restored"); ok(!highlighters.gridHighlighters.size, "CSS grid highlighter is hidden."); ok(!highlighters.state.grids.size, "No grids to be restored on page reload."); }); diff --git a/devtools/client/inspector/grids/test/browser_grids_restored-multiple-grids-after-reload.js b/devtools/client/inspector/grids/test/browser_grids_restored-multiple-grids-after-reload.js index 6740e7cb4858..42076b552100 100644 --- a/devtools/client/inspector/grids/test/browser_grids_restored-multiple-grids-after-reload.js +++ b/devtools/client/inspector/grids/test/browser_grids_restored-multiple-grids-after-reload.js @@ -120,7 +120,7 @@ add_task(async function() { ); const onStateRestored = waitForNEvents( highlighters, - "grid-state-restored", + "highlighter-restored", 3 ); const onGridListRestored = waitUntilState( @@ -137,13 +137,12 @@ add_task(async function() { state.grids[3].disabled ); await refreshTab(); - const { restored } = await onStateRestored; + await onStateRestored; await onGridListRestored; info( "Check that the grid highlighters can be displayed after reloading the page" ); - ok(restored, "The highlighter state was restored"); is( highlighters.gridHighlighters.size, 3, diff --git a/devtools/client/inspector/rules/test/browser_rules_flexbox-highlighter-restored-after-reload.js b/devtools/client/inspector/rules/test/browser_rules_flexbox-highlighter-restored-after-reload.js index aa6a4cc63315..765041604a49 100644 --- a/devtools/client/inspector/rules/test/browser_rules_flexbox-highlighter-restored-after-reload.js +++ b/devtools/client/inspector/rules/test/browser_rules_flexbox-highlighter-restored-after-reload.js @@ -30,8 +30,10 @@ add_task(async function() { const { inspector, view } = await openRuleView(); const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.FLEXBOX; const { - getNodeForActiveHighlighter, + getActiveHighlighter, waitForHighlighterTypeShown, + waitForHighlighterTypeRestored, + waitForHighlighterTypeDiscarded, } = getHighlighterTestHelpers(inspector); await selectNode("#flex", inspector); @@ -44,40 +46,24 @@ add_task(async function() { const onHighlighterShown = waitForHighlighterTypeShown(HIGHLIGHTER_TYPE); flexboxToggle.click(); await onHighlighterShown; - - ok( - getNodeForActiveHighlighter(HIGHLIGHTER_TYPE), - "Flexbox highlighter is shown." - ); + ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter is shown."); info("Reload the page, expect the highlighter to be displayed once again"); - let onStateRestored = inspector.highlighters.once("flexbox-state-restored"); - + const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE); const onReloaded = inspector.once("reloaded"); await refreshTab(); info("Wait for inspector to be reloaded after page reload"); await onReloaded; - - let { restored } = await onStateRestored; - ok(restored, "The highlighter state was restored"); - - info( - "Check that the flexbox highlighter can be displayed after reloading the page" - ); - ok( - getNodeForActiveHighlighter(HIGHLIGHTER_TYPE), - "Flexbox highlighter is shown." - ); + info("Wait for the highlighter to be restored"); + await onRestored; + ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter restored."); info("Navigate to another URL, and check that the highlighter is hidden"); const otherUri = "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI); - onStateRestored = inspector.highlighters.once("flexbox-state-restored"); + const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE); await navigateTo(otherUri); - ({ restored } = await onStateRestored); - ok(!restored, "The highlighter state was not restored"); - ok( - !getNodeForActiveHighlighter(HIGHLIGHTER_TYPE), - "Flexbox highlighter is hidden." - ); + info("Expect the highlighter not to be restored"); + await onDiscarded; + ok(!getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter not shown."); }); diff --git a/devtools/client/inspector/rules/test/browser_rules_grid-highlighter-restored-after-reload.js b/devtools/client/inspector/rules/test/browser_rules_grid-highlighter-restored-after-reload.js index 785a56e991da..a88ed04a86c4 100644 --- a/devtools/client/inspector/rules/test/browser_rules_grid-highlighter-restored-after-reload.js +++ b/devtools/client/inspector/rules/test/browser_rules_grid-highlighter-restored-after-reload.js @@ -36,6 +36,11 @@ add_task(async function() { info("Check that the grid highlighter can be displayed"); const { inspector, view } = await openRuleView(); const { highlighters } = view; + const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.GRID; + const { + waitForHighlighterTypeRestored, + waitForHighlighterTypeDiscarded, + } = getHighlighterTestHelpers(inspector); await selectNode("#grid", inspector); const container = getRuleViewProperty(view, "#grid", "display").valueSpan; @@ -49,27 +54,29 @@ add_task(async function() { is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown."); info("Reload the page, expect the highlighter to be displayed once again"); - let onStateRestored = highlighters.once("grid-state-restored"); + const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE); const onReloaded = inspector.once("reloaded"); await refreshTab(); info("Wait for inspector to be reloaded after page reload"); await onReloaded; - let { restored } = await onStateRestored; - ok(restored, "The highlighter state was restored"); - - info( - "Check that the grid highlighter can be displayed after reloading the page" + await onRestored; + is( + highlighters.gridHighlighters.size, + 1, + "CSS grid highlighter was restored." ); - is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown."); info("Navigate to another URL, and check that the highlighter is hidden"); const otherUri = "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI); - onStateRestored = highlighters.once("grid-state-restored"); + const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE); await navigateTo(otherUri); - ({ restored } = await onStateRestored); - ok(!restored, "The highlighter state was not restored"); - ok(!highlighters.gridHighlighters.size, "CSS grid highlighter is hidden."); + await onDiscarded; + is( + highlighters.gridHighlighters.size, + 0, + "CSS grid highlighter was not restored." + ); }); diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 5e47975080e2..149b2308bf8f 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -114,6 +114,9 @@ class HighlightersOverlay { // until it doesn't complete, this map will be populated with the requested type and // a unique symbol identifying that request. Once completed, the entry is removed. this._pendingHighlighters = new Map(); + // Map of highlighter types to objects with metadata used to restore active + // highlighters after a page reload. + this._restorableHighlighters = new Map(); // Collection of instantiated highlighter actors like FlexboxHighlighter, // ShapesHighlighter and GeometryEditorHighlighter. this.highlighters = {}; @@ -139,11 +142,16 @@ class HighlightersOverlay { // changes to properties in the Rule view. this.editors = {}; - // Saved state to be restore on page navigation. + // Highlighter state. this.state = { - flexbox: {}, // Map of grid container NodeFront to the their stored grid options + // Used to restore grid highlighters on reload (should be migrated to + // _restorableHighlighters in Bug 1572652). grids: new Map(), + // Shape Path Editor highlighter options. + // Used as a cache for the latest configuration when showing the highlighter. + // It is reused and augmented when hovering coordinates in the Rules view which + // mark the corresponding points in the highlighter overlay. shapes: {}, }; @@ -181,6 +189,10 @@ class HighlightersOverlay { this.showHighlighterTypeForNode.bind(this), () => this.destroyed ); + this.restoreState = safeAsyncMethod( + this.restoreState.bind(this), + () => this.destroyed + ); // Add inspector events, not specific to a given view. this.inspector.on("markupmutation", this.onMarkupMutation); @@ -214,6 +226,7 @@ class HighlightersOverlay { */ async _afterShowHighlighterTypeForNode(type, nodeFront, options) { // Log telemetry for showing the flexbox highlighter. + // Set metadata necessary to restore the active highlighter upon page refresh. if (type === TYPES.FLEXBOX) { this.telemetry.toolOpened( "FLEXBOX_HIGHLIGHTER", @@ -230,6 +243,16 @@ class HighlightersOverlay { if (scalars[options.trigger]) { this.telemetry.scalarAdd(scalars[options.trigger], 1); } + + const { url } = this.target; + const selectors = [...this.inspector.selectionCssSelectors]; + + this._restorableHighlighters.set(type, { + options, + selectors, + type, + url, + }); } } @@ -310,6 +333,9 @@ class HighlightersOverlay { * @return {Promise} */ async _beforeHideHighlighterType(type) { + // Remove any metadata used to restore this highlighter type on page refresh. + this._restorableHighlighters.delete(type); + // Log telemetry for hiding the flexbox highlighter. if (type === TYPES.FLEXBOX) { this.telemetry.toolClosed( @@ -776,28 +802,13 @@ class HighlightersOverlay { trigger, color, }); - - try { - // Save flexbox highlighter state. - const { url } = this.target; - const selector = await node.getUniqueSelector(); - this.state.flexbox = { selector, options, url }; - } catch (e) { - this._handleRejection(e); - } } /** - * Hide the flexbox highlighter for the given flexbox container element. - * - * @param {NodeFront} node - * The NodeFront of the flexbox container element to unhighlight. + * Hide the flexbox highlighter if any instance is visible. */ - async hideFlexboxHighlighter(node) { + async hideFlexboxHighlighter() { await this.hideHighlighterType(TYPES.FLEXBOX); - - // Erase flexbox highlighter state. - this.state.flexbox = null; } /** @@ -923,8 +934,8 @@ class HighlightersOverlay { try { // Save grid highlighter state. const { url } = this.target; - const selector = await node.getUniqueSelector(); - this.state.grids.set(node, { selector, options, url }); + const selectors = await node.getAllSelectors(); + this.state.grids.set(node, { selectors, options, url }); // Emit the NodeFront of the grid container element that the grid highlighter was // shown for, and its options for testing the highlighter setting options. @@ -1119,15 +1130,13 @@ class HighlightersOverlay { * Restores the saved flexbox highlighter state. */ async restoreFlexboxState() { - try { - await this.restoreState( - "flexbox", - this.state.flexbox, - this.showFlexboxHighlighter - ); - } catch (e) { - this._handleRejection(e); + const state = this._restorableHighlighters.get(TYPES.FLEXBOX); + if (!state) { + return; } + + this._restorableHighlighters.delete(TYPES.FLEXBOX); + await this.restoreState(TYPES.FLEXBOX, state, this.showFlexboxHighlighter); } /** @@ -1142,7 +1151,11 @@ class HighlightersOverlay { try { for (const gridState of values) { - await this.restoreState("grid", gridState, this.showGridHighlighter); + await this.restoreState( + TYPES.GRID, + gridState, + this.showGridHighlighter + ); } } catch (e) { this._handleRejection(e); @@ -1154,36 +1167,42 @@ class HighlightersOverlay { * Restores the saved highlighter state for the given highlighter * and their state. * - * @param {String} name - * The name of the highlighter to be restored + * @param {String} type + * Highlighter type to be restored. * @param {Object} state - * The state of the highlighter to be restored + * Object containing the metadata used to restore the highlighter. + * {Array} state.selectors + * Array of CSS selector which identifies the node to be highlighted. + * If the node is in the top-level document, the array contains just one item. + * Otherwise, if the node is nested within a stack of iframes, each iframe is + * identified by its unique selector; the last item in the array identifies + * the target node within its host iframe document. + * {Object} state.options + * Configuration options to use when showing the highlighter. + * {String} state.url + * URL of the top-level target when the metadata was stored. Used to identify + * if there was a page refresh or a navigation away to a different page. * @param {Function} showFunction * The function that shows the highlighter - * @return {Promise} that resolves when the highlighter state was restored, and the - * expected highlighters are displayed. + * @return {Promise} that resolves when the highlighter was restored and shown. */ - async restoreState(name, state, showFunction) { - const { selector, options, url } = state; + async restoreState(type, state, showFunction) { + const { selectors = [], options, url } = state; - if (!selector || url !== this.target.url) { + if (!selectors.length || url !== this.target.url) { // Bail out if no selector was saved, or if we are on a different page. - this.emit(`${name}-state-restored`, { restored: false }); + this.emit(`highlighter-discarded`, { type }); return; } - const rootNode = await this.walker.getRootNode(); - const nodeFront = await this.walker.querySelector(rootNode, selector); + const inspectorFront = await this.target.getFront("inspector"); + const nodeFront = await inspectorFront.walker.findNodeFront(selectors); if (nodeFront) { - if (options.hoverPoint) { - options.hoverPoint = null; - } - await showFunction(nodeFront, options); - this.emit(`${name}-state-restored`, { restored: true }); + this.emit(`highlighter-restored`, { type }); } else { - this.emit(`${name}-state-restored`, { restored: false }); + this.emit(`highlighter-discarded`, { type }); } } diff --git a/devtools/client/inspector/test/head.js b/devtools/client/inspector/test/head.js index a615f9d339c1..b302316dc1cf 100644 --- a/devtools/client/inspector/test/head.js +++ b/devtools/client/inspector/test/head.js @@ -826,6 +826,12 @@ function getHighlighterTestHelpers(inspector) { waitForHighlighterTypeHidden(type) { return _waitForHighlighterTypeEvent(type, "highlighter-hidden"); }, + waitForHighlighterTypeRestored(type) { + return _waitForHighlighterTypeEvent(type, "highlighter-restored"); + }, + waitForHighlighterTypeDiscarded(type) { + return _waitForHighlighterTypeEvent(type, "highlighter-discarded"); + }, }; }