diff --git a/devtools/client/inspector/rules/rules.js b/devtools/client/inspector/rules/rules.js index 39a063fb73ad..61ffd3fd0131 100644 --- a/devtools/client/inspector/rules/rules.js +++ b/devtools/client/inspector/rules/rules.js @@ -164,6 +164,11 @@ function CssRuleView(inspector, document, store) { this.refreshPanel = this.refreshPanel.bind(this); const doc = this.styleDocument; + // Delegate bulk handling of events happening within the DOM tree of the Rules view + // to this.handleEvent(). Listening on the capture phase of the event bubbling to be + // able to stop event propagation on a case-by-case basis and prevent event target + // ancestor nodes from handling them. + this.styleDocument.addEventListener("click", this, { capture: true }); this.element = doc.getElementById("ruleview-container-focusable"); this.addRuleButton = doc.getElementById("ruleview-add-rule-button"); this.searchField = doc.getElementById("ruleview-searchbox"); @@ -181,6 +186,16 @@ function CssRuleView(inspector, document, store) { this.searchClearButton.hidden = true; + this.onHighlighterShown = data => + this.handleHighlighterEvent("highlighter-shown", data); + this.onHighlighterHidden = data => + this.handleHighlighterEvent("highlighter-hidden", data); + this.inspector.highlighters.on("highlighter-shown", this.onHighlighterShown); + this.inspector.highlighters.on( + "highlighter-hidden", + this.onHighlighterHidden + ); + this.shortcuts = new KeyShortcuts({ window: this.styleWindow }); this._onShortcut = this._onShortcut.bind(this); this.shortcuts.on("Escape", event => this._onShortcut("Escape", event)); @@ -305,35 +320,7 @@ CssRuleView.prototype = { }, /** - * Get an instance of SelectorHighlighter (used to highlight nodes that match - * selectors in the rule-view). A new instance is only created the first time - * this function is called. The same instance will then be returned. - * - * @return {Promise} Resolves to the instance of the highlighter. - */ - async getSelectorHighlighter() { - if (!this.inspector) { - return null; - } - - if (this.selectorHighlighter) { - return this.selectorHighlighter; - } - - try { - const front = this.inspector.inspectorFront; - const h = await front.getHighlighterByType("SelectorHighlighter"); - this.selectorHighlighter = h; - return h; - } catch (e) { - // The SelectorHighlighter type could not be created in the - // current target. It could be an older server, or a XUL page. - return null; - } - }, - - /** - * Highlight/unhighlight all the nodes that match a given set of selectors + * Highlight/unhighlight all the nodes that match a given selector * inside the document of the current selected node. * Only one selector can be highlighted at a time, so calling the method a * second time with a different selector will first unhighlight the previously @@ -341,43 +328,24 @@ CssRuleView.prototype = { * Calling the method a second time with the same selector will just * unhighlight the highlighted nodes. * - * @param {DOMNode} selectorIcon - * The icon that was clicked to toggle the selector. The - * class 'highlighted' will be added when the selector is - * highlighted. * @param {String} selector - * The selector used to find nodes in the page. + * Elements matching this selector will be highlighted on the page. */ - async toggleSelectorHighlighter(selectorIcon, selector) { - if (this.lastSelectorIcon) { - this.lastSelectorIcon.classList.remove("highlighted"); - } - selectorIcon.classList.remove("highlighted"); - - const highlighter = await this.getSelectorHighlighter(); - if (!highlighter) { - return; - } - - await highlighter.hide(); - - if (selector !== this.highlighters.selectorHighlighterShown) { - this.highlighters.selectorHighlighterShown = selector; - selectorIcon.classList.add("highlighted"); - this.lastSelectorIcon = selectorIcon; - - const node = this.inspector.selection.nodeFront; - - await highlighter.show(node, { - hideInfoBar: true, - hideGuides: true, - selector, - }); - - this.emit("ruleview-selectorhighlighter-toggled", true); + async toggleSelectorHighlighter(selector) { + if (this.isSelectorHighlighted(selector)) { + await this.inspector.highlighters.hideHighlighterType( + this.inspector.highlighters.TYPES.SELECTOR + ); } else { - this.highlighters.selectorHighlighterShown = null; - this.emit("ruleview-selectorhighlighter-toggled", false); + await this.inspector.highlighters.showHighlighterTypeForNode( + this.inspector.highlighters.TYPES.SELECTOR, + this.inspector.selection.nodeFront, + { + hideInfoBar: true, + hideGuides: true, + selector, + } + ); } }, @@ -393,6 +361,89 @@ CssRuleView.prototype = { ); }, + /** + * Check whether a SelectorHighlighter is active for the given selector text. + * + * @param {String} selector + * @return {Boolean} + */ + isSelectorHighlighted(selector) { + const options = this.inspector.highlighters.getOptionsForActiveHighlighter( + this.inspector.highlighters.TYPES.SELECTOR + ); + + return options?.selector === selector; + }, + + /** + * Delegate handler for events happening within the DOM tree of the Rules view. + * Itself delegates to specific handlers by event type. + * + * Use this instead of attaching specific event handlers when: + * - there are many elements with the same event handler (eases memory pressure) + * - you want to avoid having to remove event handlers manually + * - elements are added/removed from the DOM tree arbitrarily over time + * + * @param {MouseEvent|UIEvent} event + */ + handleEvent(event) { + switch (event.type) { + case "click": + this.handleClickEvent(event); + break; + default: + } + }, + + /** + * Delegate handler for click events happening within the DOM tree of the Rules view. + * + * @param {MouseEvent} event + */ + handleClickEvent(event) { + const target = event.target; + + // Handle click on the icon next to a CSS selector. + if (target.classList.contains("js-toggle-selector-highlighter")) { + this.toggleSelectorHighlighter(target.dataset.selector); + // Prevent the click on the element wrapping the CSS rule + // from triggering the prompt to add a new CSS declaration + event.stopPropagation(); + } + }, + + /** + * Delegate handler for highlighter events. + * + * This is the place to observe for highlighter events, check the highlighter type and + * event name, then react to specific events, for example by modifying the DOM. + * + * @param {String} eventName + * Highlighter event name. One of: "highlighter-hidden", "highlighter-shown" + * @param {Object} data + * Object with data associated with the highlighter event. + */ + handleHighlighterEvent(eventName, data) { + switch (data.type) { + // Toggle the "highlighted" CSS class name on selector icons in the Rules view when + // the SelectorHighlighter is shown/hidden for a certain CSS selector. + case this.inspector.highlighters.TYPES.SELECTOR: + if (data?.options?.selector) { + const selector = data?.options?.selector; + const query = `.js-toggle-selector-highlighter[data-selector='${selector}']`; + for (const node of this.styleDocument.querySelectorAll(query)) { + if (eventName == "highlighter-hidden") { + node.classList.remove("highlighted"); + } + if (eventName == "highlighter-shown") { + node.classList.add("highlighted"); + } + } + } + break; + } + }, + /** * Initializes the content-viewer front and enable the print and color scheme simulation * if they are supported in the current target. @@ -756,6 +807,7 @@ CssRuleView.prototype = { // Remove bound listeners this.shortcuts.destroy(); + this.styleDocument.removeEventListener("click", this); this.element.removeEventListener("copy", this._onCopy); this.element.removeEventListener("contextmenu", this._onContextMenu); this.addRuleButton.removeEventListener("click", this._onAddRule); @@ -770,6 +822,14 @@ CssRuleView.prototype = { this._onTogglePseudoClassPanel ); this.classToggle.removeEventListener("click", this._onToggleClassPanel); + this.inspector.highlighters.off( + "highlighter-shown", + this.onHighlighterShown + ); + this.inspector.highlighters.off( + "highlighter-hidden", + this.onHighlighterHidden + ); this.searchField = null; this.searchClearButton = null; @@ -1033,8 +1093,6 @@ CssRuleView.prototype = { * Clear the rule view. */ clear: function(clearDom = true) { - this.lastSelectorIcon = null; - if (clearDom) { this._clearRules(); } diff --git a/devtools/client/inspector/rules/views/rule-editor.js b/devtools/client/inspector/rules/views/rule-editor.js index a917d1156b14..977f900ababc 100644 --- a/devtools/client/inspector/rules/views/rule-editor.js +++ b/devtools/client/inspector/rules/views/rule-editor.js @@ -159,6 +159,7 @@ RuleEditor.prototype = { } if (this.rule.domRule.type !== CSSRule.KEYFRAME_RULE) { + // FIXME: Avoid having this as a nested async operation. (Bug 1664511) (async function() { let selector; @@ -174,26 +175,15 @@ RuleEditor.prototype = { selector = this.ruleView.inspector.selectionCssSelector; } - const isHighlighted = - this.ruleView._highlighters && - this.ruleView.highlighters.selectorHighlighterShown === selector; - const selectorHighlighter = createChild(header, "span", { + const isHighlighted = this.ruleView.isSelectorHighlighted(selector); + // Handling of click events is delegated to CssRuleView.handleEvent() + createChild(header, "span", { class: - "ruleview-selectorhighlighter" + + "ruleview-selectorhighlighter js-toggle-selector-highlighter" + (isHighlighted ? " highlighted" : ""), + "data-selector": selector, title: l10n("rule.selectorHighlighter.tooltip"), }); - selectorHighlighter.addEventListener("click", event => { - this.ruleView.toggleSelectorHighlighter( - selectorHighlighter, - selector - ); - // Prevent clicks from focusing the property editor. - event.stopPropagation(); - }); - - this.uniqueSelector = selector; - this.emit("selector-icon-created"); } .bind(this)() .catch(error => { @@ -668,6 +658,11 @@ RuleEditor.prototype = { this.isEditing = true; + // Remove highlighter for the previous selector. + if (this.ruleView.isSelectorHighlighted(this.rule.selectorText)) { + await this.ruleView.toggleSelectorHighlighter(this.rule.selectorText); + } + try { const response = await this.rule.domRule.modifySelector(element, value); @@ -715,14 +710,6 @@ RuleEditor.prototype = { // pseudo-element rules and the like. this.element.parentNode.replaceChild(editor.element, this.element); - // Remove highlight for modified selector - if (ruleView.highlighters.selectorHighlighterShown) { - ruleView.toggleSelectorHighlighter( - ruleView.lastSelectorIcon, - ruleView.highlighters.selectorHighlighterShown - ); - } - editor._moveSelectorFocus(direction); } catch (err) { this.isEditing = false; diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 244f85918c7f..da0b988fe3ca 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -157,8 +157,6 @@ class HighlightersOverlay { this.geometryEditorHighlighterShown = null; // Name of the highlighter shown on mouse hover. this.hoveredHighlighterShown = null; - // Name of the selector highlighter shown. - this.selectorHighlighterShown = null; // NodeFront of the shape that is highlighted this.shapesHighlighterShown = null; @@ -205,6 +203,74 @@ class HighlightersOverlay { EventEmitter.decorate(this); } + /** + * Optionally run some operations before showing a highligther of a given type. + * + * Depending its type, before showing a new instance of a highlighter, we may do extra + * operations, like hiding another visible highlighter, or preventing the show + * operation, for example due to a duplicate call with the same arguments. + * + * Returns a promise that resovles with a boolean indicating whether to skip showing + * the highlighter with these arguments. + * + * @param {String} type + * Highlighter type to show. + * @param {NodeFront} nodeFront + * Node front of the element to be highlighted. + * @param {Options} options + * Optional object with options to pass to the highlighter. + * @return {Promise} + */ + async _beforeShowHighlighterTypeForNode(type, nodeFront, options) { + // Get the data associated with the visible highlighter of this type, if any. + const { + highlighter: activeHighlighter, + nodeFront: activeNodeFront, + options: activeOptions, + timer: activeTimer, + } = this.getDataForActiveHighlighter(type); + + // There isn't an active highlighter of this type. Early return, proceed with showing. + if (!activeHighlighter) { + return false; + } + + // Whether conditions are met to skip showing the highlighter (ex: duplicate calls). + let skipShow = false; + + // Clear any autohide timer associated with this highlighter type. + // This clears any existing timer for duplicate calls to show() if: + // - called with different options.duration + // - called once with options.duration, then without (see deepEqual() above) + clearTimeout(activeTimer); + + switch (type) { + // Hide the visible selector highlighter if called for the same node, + // but with a different selector. + case TYPES.SELECTOR: + if ( + nodeFront === activeNodeFront && + options?.selector !== activeOptions?.selector + ) { + await this.hideHighlighterType(TYPES.SELECTOR); + } + break; + + // For others, hide the existing highlighter before showing it for a different node. + // Else, if the node is the same and options are the same, skip a duplicate call. + // Duplicate calls to show the highlighter for the same node are allowed + // if the options are different (for example, when scheduling autohide). + default: + if (nodeFront !== activeNodeFront) { + await this.hideHighlighterType(type); + } else if (deepEqual(options, activeOptions)) { + skipShow = true; + } + } + + return skipShow; + } + /** * Get a highlighter instance of the given type for the given node front. * @@ -239,6 +305,39 @@ class HighlightersOverlay { return highlighter; } + /** + * Get an object with data associated with the active highlighter of a given type. + * This data object contains: + * - nodeFront: NodeFront of the highlighted node + * - highlighter: Highlighter instance + * - options: Configuration options passed to the highlighter + * - timer: (Optional) index of timer set with setTimout() to autohide the highlighter + * Returns an empty object if a highlighter of the given type is not active. + * + * @param {String} type + * Highlighter type. + * @return {Object} + */ + getDataForActiveHighlighter(type) { + if (!this._activeHighlighters.has(type)) { + return {}; + } + + return this._activeHighlighters.get(type); + } + + /** + * Get the configuration options of the active highlighter of a given type. + * + * @param {String} type + * Highlighter type. + * @return {Object} + */ + getOptionsForActiveHighlighter(type) { + const { options } = this.getDataForActiveHighlighter(type); + return options; + } + /** * Get the node front highlighted by a given highlighter type. * @@ -274,28 +373,14 @@ class HighlightersOverlay { * @return {Promise} */ async showHighlighterTypeForNode(type, nodeFront, options) { - if (this._activeHighlighters.has(type)) { - const { - nodeFront: activeNodeFront, - options: activeOptions, - timer: activeTimer, - } = this._activeHighlighters.get(type); + const skipShow = await this._beforeShowHighlighterTypeForNode( + type, + nodeFront, + options + ); - // Hide the existing highlighter before showing it for a different node. - // Else, if the node is the same and options are the same, skip duplicate call. - // Duplicate calls to show the highlighter for the same node are allowed - // if the options are different (for example, when scheduling autohide). - if (nodeFront !== activeNodeFront) { - await this.hideHighlighterType(type); - } else if (deepEqual(options, activeOptions)) { - return; - } - - // Clear any autohide timer associated with this highlighter type. - // This clears any existing timer for duplicate calls to show() if: - // - called with different options.duration - // - called once with options.duration, then without (see deepEqual() above) - clearTimeout(activeTimer); + if (skipShow) { + return; } const highlighter = await this._getHighlighterTypeForNode(type, nodeFront); @@ -354,9 +439,8 @@ class HighlightersOverlay { return; } - const { highlighter, nodeFront, timer } = this._activeHighlighters.get( - type - ); + const data = this.getDataForActiveHighlighter(type); + const { highlighter, nodeFront, timer } = data; // Clear any autohide timer associated with this highlighter type. clearTimeout(timer); this._activeHighlighters.delete(type); @@ -367,7 +451,7 @@ class HighlightersOverlay { if (HIGHLIGHTER_EVENTS[type]?.hidden) { this.emit(HIGHLIGHTER_EVENTS[type].hidden, nodeFront); } - this.emit("highlighter-hidden", { type, highlighter, nodeFront }); + this.emit("highlighter-hidden", { type, ...data }); } async canGetParentGridNode() { @@ -1634,7 +1718,6 @@ class HighlightersOverlay { this.flexboxHighlighterShown = null; this.geometryEditorHighlighterShown = null; this.hoveredHighlighterShown = null; - this.selectorHighlighterShown = null; this.shapesHighlighterShown = null; } @@ -1732,7 +1815,6 @@ class HighlightersOverlay { this.flexboxHighlighterShown = null; this.geometryEditorHighlighterShown = null; this.hoveredHighlighterShown = null; - this.selectorHighlighterShown = null; this.shapesHighlighterShown = null; this.destroyed = true;