commit b653b2c342f313bd3b89cf1d2709ce1a499c6051 parent c82adde571c24d89da3ef13f11245c8f572ba1e6 Author: Nicolas Chevobbe <nchevobbe@mozilla.com> Date: Thu, 13 Nov 2025 09:46:36 +0000 Bug 1997847 - [devtools] Cleanup pseudo element handling in inspector client and server. r=devtools-reviewers,ochameau We used to compute the pseudo element name (e.g. `::before`, `::marker`, …) in a few places, both on the server and the client. This patch does it directly from `getNodeDisplayName`, which is called for assigning the NodeActorForm displayName. We also use `getNodeDisplayName` in `getBindingElementAndPseudo` to avoid duplicating code. On the client we now use displayName in the few places we used to re-construct the pseudo element text in (markup view, breadcrumb, rules view, …) We also no longer pass "individual" pseudo element booleans (e.g. `isAfterPseudoElement`) in the NodeActor form, but a more generic `isPseudoElement` one so it's easier to handle future pseudo element (e.g. `::view-transition`). And we take this as an opportunity to remove the utility functions that we were using to build those properties, and instead switch to look into `Node.implementedPseudoElement` to detect when we do handle pseudo elements. Differential Revision: https://phabricator.services.mozilla.com/D271002 Diffstat:
18 files changed, 136 insertions(+), 209 deletions(-)
diff --git a/devtools/client/fronts/node.js b/devtools/client/fronts/node.js @@ -289,6 +289,22 @@ class NodeFront extends FrontClassWithSpec(nodeSpec) { return this._form.nodeName; } get displayName() { + // @backward-compat { version 147 } When 147 reaches release, we can remove this 'if' block. + // The form's displayName will be populated correctly for pseudo elements. + if ( + this.isPseudoElement && + !this.traits.hasPseudoElementNameInDisplayName + ) { + if (this.isMarkerPseudoElement) { + return "::marker"; + } + if (this.isBeforePseudoElement) { + return "::before"; + } + if (this.isAfterPseudoElement) { + return "::after"; + } + } const { displayName, nodeName } = this._form; // Keep `nodeName.toLowerCase()` for backward compatibility @@ -369,11 +385,18 @@ class NodeFront extends FrontClassWithSpec(nodeSpec) { return this._form.isAfterPseudoElement; } get isPseudoElement() { - return ( - this.isBeforePseudoElement || - this.isAfterPseudoElement || - this.isMarkerPseudoElement - ); + // @backward-compat { version 147 } When 147 reaches release, we can remove this 'if' block, + // as well as the isXXXPseudoElement getters. + // The form isPseudoElement property will be populated correctly. + if (!this.traits.hasPseudoElementNameInDisplayName) { + return ( + this.isBeforePseudoElement || + this.isAfterPseudoElement || + this.isMarkerPseudoElement + ); + } + + return this._form.isPseudoElement; } get isAnonymous() { return this._form.isAnonymous; diff --git a/devtools/client/inspector/breadcrumbs.js b/devtools/client/inspector/breadcrumbs.js @@ -435,31 +435,26 @@ HTMLBreadcrumbs.prototype = { /** * Build a string that represents the node: tagName#id.class1.class2. * - * @param {NodeFront} node The node to pretty-print + * @param {NodeFront} nodeFront The node to pretty-print * @return {String} */ - prettyPrintNodeAsText(node) { - let text = node.isShadowRoot ? SHADOW_ROOT_TAGNAME : node.displayName; - if (node.isMarkerPseudoElement) { - text = "::marker"; - } else if (node.isBeforePseudoElement) { - text = "::before"; - } else if (node.isAfterPseudoElement) { - text = "::after"; - } + prettyPrintNodeAsText(nodeFront) { + let text = nodeFront.isShadowRoot + ? SHADOW_ROOT_TAGNAME + : nodeFront.displayName; - if (node.id) { - text += "#" + node.id; + if (nodeFront.id) { + text += "#" + nodeFront.id; } - if (node.className) { - const classList = node.className.split(/\s+/); + if (nodeFront.className) { + const classList = nodeFront.className.split(/\s+/); for (let i = 0; i < classList.length; i++) { text += "." + classList[i]; } } - for (const pseudo of node.pseudoClassLocks) { + for (const pseudo of nodeFront.pseudoClassLocks) { text += pseudo; } @@ -492,13 +487,6 @@ HTMLBreadcrumbs.prototype = { pseudosLabel.className = "breadcrumbs-widget-item-pseudo-classes"; let tagText = node.isShadowRoot ? SHADOW_ROOT_TAGNAME : node.displayName; - if (node.isMarkerPseudoElement) { - tagText = "::marker"; - } else if (node.isBeforePseudoElement) { - tagText = "::before"; - } else if (node.isAfterPseudoElement) { - tagText = "::after"; - } let idText = node.id ? "#" + node.id : ""; let classesText = ""; diff --git a/devtools/client/inspector/grids/test/browser_grids_grid-list-element-rep.js b/devtools/client/inspector/grids/test/browser_grids_grid-list-element-rep.js @@ -79,7 +79,7 @@ add_task(async function () { nodeFront = await highlightAndSelectNode(inspector, pseudoRep); is( nodeFront.displayName, - "_moz_generated_content_after", + "::after", "The expected node was highlighted/selected" ); is( diff --git a/devtools/client/inspector/markup/markup.js b/devtools/client/inspector/markup/markup.js @@ -2834,13 +2834,13 @@ class MarkupView extends EventEmitter { if (nextSibling) { while ( - nextSibling.isMarkerPseudoElement || - nextSibling.isBeforePseudoElement + nextSibling.displayName === "::marker" || + nextSibling.displayName === "::before" ) { nextSibling = this.getContainer(nextSibling).elt.nextSibling.container.node; } - if (nextSibling.isAfterPseudoElement) { + if (nextSibling.displayName === "::after") { parent = target.parentNode.container.node.parentNode(); nextSibling = null; } diff --git a/devtools/client/inspector/markup/test/browser_markup_dragdrop_before_marker_pseudo.js b/devtools/client/inspector/markup/test/browser_markup_dragdrop_before_marker_pseudo.js @@ -26,11 +26,7 @@ add_task(async function () { info("Test placing an element before a ::marker psuedo"); await moveElementBeforeMarker("#last-list-child", parentFront, inspector); const childNodes = await getChildrenOf(parentFront, inspector); - is( - childNodes[0], - "_moz_generated_content_marker", - "::marker is still the first child of #list" - ); + is(childNodes[0], "::marker", "::marker is still the first child of #list"); is( childNodes[1], "last-list-child", @@ -70,7 +66,7 @@ async function moveElementBeforeMarker(selector, parentFront, inspector) { async function getChildrenOf(parentFront, { walker }) { const { nodes } = await walker.children(parentFront); return nodes.map(node => { - if (node.isMarkerPseudoElement) { + if (node.displayName === "::marker") { return node.displayName; } return node.id; diff --git a/devtools/client/inspector/markup/test/browser_markup_pseudo.js b/devtools/client/inspector/markup/test/browser_markup_pseudo.js @@ -44,7 +44,7 @@ add_task(async function testMarkerOnPseudo() { const ulBeforeNodeFront = ulChildren[0]; is( ulBeforeNodeFront.displayName, - "_moz_generated_content_before", + "::before", "Got expexected ul::before pseudo element" ); const ulBeforeContainer = await getContainerForNodeFront( @@ -64,7 +64,7 @@ add_task(async function testMarkerOnPseudo() { const ulBeforeMarkerNodeFront = ulBeforeChildren[0]; is( ulBeforeMarkerNodeFront.displayName, - "_moz_generated_content_marker", + "::marker", "Got expexected ul::before::marker pseudo element" ); const ulBeforeMarkerContainer = await getContainerForNodeFront( @@ -81,7 +81,7 @@ add_task(async function testMarkerOnPseudo() { const ulAfterNodeFront = ulChildren[3]; is( ulAfterNodeFront.displayName, - "_moz_generated_content_after", + "::after", "Got expexected ul::after pseudo element" ); const ulAfterContainer = await getContainerForNodeFront( @@ -104,13 +104,15 @@ async function checkMarkupView(inspector) { const divNode = childrenContainers[1].node; const afterNode = childrenContainers[2].node; - ok( - beforeNode.isBeforePseudoElement, + is( + beforeNode.displayName, + "::before", "The first child is the ::before pseudo element" ); is(divNode.displayName, "div", "The second child is the <div> element"); - ok( - afterNode.isAfterPseudoElement, + is( + afterNode.displayName, + "::after", "The last child is the ::after pseudo element" ); } diff --git a/devtools/client/inspector/markup/views/read-only-editor.js b/devtools/client/inspector/markup/views/read-only-editor.js @@ -9,27 +9,21 @@ const nodeConstants = require("resource://devtools/shared/dom-node-constants.js" /** * Creates an editor for non-editable nodes. */ -function ReadOnlyEditor(container, node) { +function ReadOnlyEditor(container, nodeFront) { this.container = container; this.markup = this.container.markup; this.buildMarkup(); - if (node.isPseudoElement) { - this.tag.classList.add("pseudo", "force-color-on-flash"); - if (node.isMarkerPseudoElement) { - this.tag.textContent = "::marker"; - } else if (node.isBeforePseudoElement) { - this.tag.textContent = "::before"; - } else if (node.isAfterPseudoElement) { - this.tag.textContent = "::after"; - } - } else if (node.nodeType == nodeConstants.DOCUMENT_TYPE_NODE) { + if (nodeFront.nodeType == nodeConstants.DOCUMENT_TYPE_NODE) { this.elt.classList.add("comment", "doctype"); - this.tag.textContent = node.doctypeString; - } else if (node.isShadowRoot) { - this.tag.textContent = `#shadow-root (${node.shadowRootMode})`; + this.tag.textContent = nodeFront.doctypeString; + } else if (nodeFront.isShadowRoot) { + this.tag.textContent = `#shadow-root (${nodeFront.shadowRootMode})`; } else { - this.tag.textContent = node.nodeName; + this.tag.textContent = nodeFront.displayName; + if (nodeFront.isPseudoElement) { + this.tag.classList.add("pseudo", "force-color-on-flash"); + } } // Make the "tag" part of this editor focusable. diff --git a/devtools/client/inspector/shared/utils.js b/devtools/client/inspector/shared/utils.js @@ -148,23 +148,8 @@ async function getLongString(longStringActorPromise) { * @return {String} selector of the element node. */ function getSelectorFromGrip(grip) { - const { - attributes, - nodeName, - isAfterPseudoElement, - isBeforePseudoElement, - isMarkerPseudoElement, - } = grip.preview; - - if (isAfterPseudoElement) { - return "::after"; - } else if (isBeforePseudoElement) { - return "::before"; - } else if (isMarkerPseudoElement) { - return "::marker"; - } - - let selector = nodeName; + const { attributes, displayName } = grip.preview; + let selector = displayName; if (attributes.id) { selector += `#${attributes.id}`; @@ -217,11 +202,10 @@ function translateNodeFrontToGrip(nodeFront) { preview: { attributes: attributesMap, attributesLength: attributes.length, - isAfterPseudoElement: nodeFront.isAfterPseudoElement, - isBeforePseudoElement: nodeFront.isBeforePseudoElement, - isMarkerPseudoElement: nodeFront.isMarkerPseudoElement, + isPseudoElement: nodeFront.isPseudoElement, // All the grid containers are assumed to be in the DOM tree. isConnected: true, + displayName: nodeFront.displayName, // nodeName is already lowerCased in Node grips nodeName: nodeFront.nodeName.toLowerCase(), nodeType: nodeFront.nodeType, diff --git a/devtools/client/inspector/test/highlighter/browser_inspector_highlighter-02.js b/devtools/client/inspector/test/highlighter/browser_inspector_highlighter-02.js @@ -52,7 +52,7 @@ add_task(async function () { const ulBeforeNodeFront = ulChildren[0]; is( ulBeforeNodeFront.displayName, - "_moz_generated_content_before", + "::before", "Got expexected ul::before pseudo element" ); @@ -74,7 +74,7 @@ add_task(async function () { const ulBeforeMarkerNodeFront = ulBeforeChildren[0]; is( ulBeforeMarkerNodeFront.displayName, - "_moz_generated_content_marker", + "::marker", "Got expexected ul::before::marker pseudo element" ); diff --git a/devtools/client/shared/components/reps/reps/element-node.mjs b/devtools/client/shared/components/reps/reps/element-node.mjs @@ -37,8 +37,7 @@ ElementNode.propTypes = { function ElementNode(props) { const { object, mode, shouldRenderTooltip } = props; - const { isAfterPseudoElement, isBeforePseudoElement, isMarkerPseudoElement } = - object.preview; + const { isPseudoElement } = object.preview; let renderElements = []; const isInTree = object.preview && object.preview.isConnected === true; @@ -46,8 +45,11 @@ function ElementNode(props) { const inspectIcon = getInspectIcon({ ...props, isInTree }); // Elements Case 1: Pseudo Element - if (isAfterPseudoElement || isBeforePseudoElement || isMarkerPseudoElement) { - const pseudoNodeElement = getPseudoNodeElement(object); + if (isPseudoElement) { + const pseudoNodeElement = { + config: { className: "attrName" }, + content: object.preview.displayName, + }; // Regenerate config if shouldRenderTooltip if (shouldRenderTooltip) { @@ -238,26 +240,6 @@ function getTinyElements(grip) { return elements; } -function getPseudoNodeElement(grip) { - const { isAfterPseudoElement, isBeforePseudoElement, isMarkerPseudoElement } = - grip.preview; - - let pseudoNodeName; - - if (isAfterPseudoElement) { - pseudoNodeName = "after"; - } else if (isBeforePseudoElement) { - pseudoNodeName = "before"; - } else if (isMarkerPseudoElement) { - pseudoNodeName = "marker"; - } - - return { - config: { className: "attrName" }, - content: `::${pseudoNodeName}`, - }; -} - function getInspectIcon(opts) { const { object, diff --git a/devtools/server/actors/inspector/event-collector.js b/devtools/server/actors/inspector/event-collector.js @@ -8,9 +8,6 @@ "use strict"; const { - isAfterPseudoElement, - isBeforePseudoElement, - isMarkerPseudoElement, isNativeAnonymous, } = require("resource://devtools/shared/layout/utils.js"); const Debugger = require("Debugger"); @@ -425,15 +422,9 @@ class JQueryEventCollector extends MainEventCollector { const jQuery = this.getJQuery(node); const handlers = []; - // If jQuery is not on the page, if this is an anonymous node or a pseudo - // element we need to return early. - if ( - !jQuery || - isNativeAnonymous(node) || - isMarkerPseudoElement(node) || - isBeforePseudoElement(node) || - isAfterPseudoElement(node) - ) { + // If jQuery is not on the page, if this is an anonymous node (that includes pseudo + // elements), we need to return early. + if (!jQuery || isNativeAnonymous(node)) { if (checkOnly) { return false; } diff --git a/devtools/server/actors/inspector/node.js b/devtools/server/actors/inspector/node.js @@ -25,13 +25,10 @@ loader.lazyRequireGetter( this, [ "getShadowRootMode", - "isAfterPseudoElement", "isAnonymous", - "isBeforePseudoElement", "isDirectShadowHostChild", "isFrameBlockedByCSP", "isFrameWithChildTarget", - "isMarkerPseudoElement", "isNativeAnonymous", "isShadowHost", "isShadowRoot", @@ -212,9 +209,7 @@ class NodeActor extends Actor { attrs: this.writeAttrs(), customElementLocation: this.getCustomElementLocation(), - isMarkerPseudoElement: isMarkerPseudoElement(this.rawNode), - isBeforePseudoElement: isBeforePseudoElement(this.rawNode), - isAfterPseudoElement: isAfterPseudoElement(this.rawNode), + isPseudoElement: !!this.rawNode.implementedPseudoElement, isAnonymous: isAnonymous(this.rawNode), isNativeAnonymous: isNativeAnonymous(this.rawNode), isShadowRoot: shadowRoot, @@ -228,7 +223,10 @@ class NodeActor extends Actor { isInHTMLDocument: this.rawNode.ownerDocument && this.rawNode.ownerDocument.contentType === "text/html", - traits: {}, + traits: { + // @backward-compat { version 147 } Can be removed once 147 reaches release + hasPseudoElementNameInDisplayName: true, + }, }; // The event collector can be expensive, so only check for events on nodes that @@ -239,9 +237,7 @@ class NodeActor extends Actor { nodeType !== Node.CDATA_SECTION_NODE && nodeType !== Node.DOCUMENT_NODE && nodeType !== Node.DOCUMENT_TYPE_NODE && - !form.isMarkerPseudoElement && - !form.isBeforePseudoElement && - !form.isAfterPseudoElement + !form.isPseudoElement ) { form.hasEventListeners = this.hasEventListeners(); } @@ -335,9 +331,7 @@ class NodeActor extends Actor { // FIXME: We should be able to just check <slot> rather than // containingShadowRoot. this.rawNode.containingShadowRoot || - isMarkerPseudoElement(this.rawNode) || - isBeforePseudoElement(this.rawNode) || - isAfterPseudoElement(this.rawNode) + !!this.rawNode.implementedPseudoElement ) { numChildren = this.walker.countChildren(this); } diff --git a/devtools/server/actors/inspector/utils.js b/devtools/server/actors/inspector/utils.js @@ -71,6 +71,11 @@ const IMAGE_FETCHING_TIMEOUT = 500; * Properly cased version of the node tag name */ const getNodeDisplayName = function (rawNode) { + const { implementedPseudoElement } = rawNode; + if (implementedPseudoElement) { + return implementedPseudoElement; + } + if (rawNode.nodeName && !rawNode.localName) { // The localName & prefix APIs have been moved from the Node interface to the Element // interface. Use Node.nodeName as a fallback. diff --git a/devtools/server/actors/inspector/walker.js b/devtools/server/actors/inspector/walker.js @@ -24,10 +24,7 @@ loader.lazyRequireGetter( this, [ "getFrameElement", - "isAfterPseudoElement", - "isBeforePseudoElement", "isDirectShadowHostChild", - "isMarkerPseudoElement", "isFrameBlockedByCSP", "isFrameWithChildTarget", "isShadowHost", @@ -737,9 +734,7 @@ class WalkerActor extends Actor { inlineTextChild(rawNode) { // Quick checks to prevent creating a new walker if possible. if ( - isMarkerPseudoElement(rawNode) || - isBeforePseudoElement(rawNode) || - isAfterPseudoElement(rawNode) || + !!rawNode.implementedPseudoElement || isShadowHost(rawNode) || rawNode.nodeType != Node.ELEMENT_NODE || !!rawNode.children.length || diff --git a/devtools/server/actors/utils/walker-search.js b/devtools/server/actors/utils/walker-search.js @@ -6,11 +6,10 @@ loader.lazyRequireGetter( this, - "isWhitespaceTextNode", + ["isWhitespaceTextNode", "getNodeDisplayName"], "resource://devtools/server/actors/inspector/utils.js", true ); - /** * The walker-search module provides a simple API to index and search strings * and elements inside a given document. @@ -104,22 +103,26 @@ class WalkerIndex { } if (node.nodeType === 1) { - // For each element node, we get the tagname and all attributes names - // and values - const localName = node.localName; - if (localName === "_moz_generated_content_marker") { - this._addToIndex("tag", node, "::marker"); - this._addToIndex("text", node, node.textContent.trim()); - } else if (localName === "_moz_generated_content_before") { - this._addToIndex("tag", node, "::before"); - this._addToIndex("text", node, node.textContent.trim()); - } else if (localName === "_moz_generated_content_after") { - this._addToIndex("tag", node, "::after"); - this._addToIndex("text", node, node.textContent.trim()); + if (node.implementedPseudoElement) { + // For pseudo elements we get the displayName (e.g. `::view-transition-group(myGroup)`) + const displayName = getNodeDisplayName(node); + this._addToIndex("tag", node, displayName); + + // And for the pseudo elements that do have text child (via the CSS `content` property), + // we also get the text. + if ( + displayName === "::marker" || + displayName === "::before" || + displayName === "::after" + ) { + this._addToIndex("text", node, node.textContent.trim()); + } } else { + // For each element node, we get the tagname … this._addToIndex("tag", node, node.localName); } + // … and all attributes names and values for (const { name, value } of node.attributes) { this._addToIndex("attributeName", node, name); this._addToIndex("attributeValue", node, value); diff --git a/devtools/server/tests/browser/browser_inspector-shadow.js b/devtools/server/tests/browser/browser_inspector-shadow.js @@ -100,12 +100,14 @@ add_task(async function () { const children = await walker.children(el); ok(children.nodes[0].isShadowRoot, "#host-pseudo 1st child is a shadow root"); - ok( - children.nodes[1].isBeforePseudoElement, + is( + children.nodes[1].displayName, + "::before", "#host-pseudo 2nd child is ::before" ); - ok( - children.nodes[2].isAfterPseudoElement, + is( + children.nodes[2].displayName, + "::after", "#host-pseudo 3rd child is ::after" ); }); @@ -122,12 +124,8 @@ add_task(async function () { is(shadowChildren.nodes[1].displayName, "slot", "shadow-root has a slot"); const slottedChildren = await walker.children(shadowChildren.nodes[1]); - ok(slottedChildren.nodes[0].isBeforePseudoElement, "slot has ::before"); - ok( - slottedChildren.nodes[slottedChildren.nodes.length - 1] - .isAfterPseudoElement, - "slot has ::after" - ); + is(slottedChildren.nodes[0].displayName, "::before", "slot has ::before"); + is(slottedChildren.nodes.at(-1).displayName, "::after", "slot has ::after"); }); add_task(async function () { diff --git a/devtools/shared/inspector/css-logic.js b/devtools/shared/inspector/css-logic.js @@ -29,6 +29,12 @@ loader.lazyRequireGetter( "resource://devtools/shared/indentation.js", true ); +loader.lazyRequireGetter( + this, + "getNodeDisplayName", + "resource://devtools/server/actors/inspector/utils.js", + true +); const { LocalizationHelper } = require("resource://devtools/shared/l10n.js"); const styleInspectorL10N = new LocalizationHelper( "devtools/shared/locales/styleinspector.properties" @@ -555,15 +561,18 @@ exports.prettifyCSS = prettifyCSS; function getBindingElementAndPseudo(node) { let bindingElement = node; let pseudo = null; - if (node.nodeName == "_moz_generated_content_marker") { - bindingElement = node.parentNode; - pseudo = "::marker"; - } else if (node.nodeName == "_moz_generated_content_before") { - bindingElement = node.parentNode; - pseudo = "::before"; - } else if (node.nodeName == "_moz_generated_content_after") { - bindingElement = node.parentNode; - pseudo = "::after"; + if (node.implementedPseudoElement) { + // we can't return `node.implementedPseudoElement` directly as the property only holds + // the pseudo element type (e.g. "::view-transition-group"), while the callsites of this + // function need the full pseudo element name (e.g. "::view-transition-group(root)") + pseudo = getNodeDisplayName(node); + if ( + pseudo === "::marker" || + pseudo === "::before" || + pseudo === "::after" + ) { + bindingElement = node.parentNode; + } } return { bindingElement, diff --git a/devtools/shared/layout/utils.js b/devtools/shared/layout/utils.js @@ -488,13 +488,9 @@ exports.isShadowHost = isShadowHost; * @return {Boolean} */ function isDirectShadowHostChild(node) { - // Pseudo elements and native anonymous elements are always part of the anonymous tree. - if ( - isMarkerPseudoElement(node) || - isBeforePseudoElement(node) || - isAfterPseudoElement(node) || - node.isNativeAnonymous - ) { + // native anonymous elements (which includes pseudo elements) are always part of the + // anonymous tree. + if (node.isNativeAnonymous) { return false; } @@ -504,39 +500,6 @@ function isDirectShadowHostChild(node) { exports.isDirectShadowHostChild = isDirectShadowHostChild; /** - * Determine whether a node is a ::marker pseudo. - * - * @param {DOMNode} node - * @return {Boolean} - */ -function isMarkerPseudoElement(node) { - return node.nodeName === "_moz_generated_content_marker"; -} -exports.isMarkerPseudoElement = isMarkerPseudoElement; - -/** - * Determine whether a node is a ::before pseudo. - * - * @param {DOMNode} node - * @return {Boolean} - */ -function isBeforePseudoElement(node) { - return node.nodeName === "_moz_generated_content_before"; -} -exports.isBeforePseudoElement = isBeforePseudoElement; - -/** - * Determine whether a node is a ::after pseudo. - * - * @param {DOMNode} node - * @return {Boolean} - */ -function isAfterPseudoElement(node) { - return node.nodeName === "_moz_generated_content_after"; -} -exports.isAfterPseudoElement = isAfterPseudoElement; - -/** * Get the current zoom factor applied to the container window of a given node. * * @param {DOMNode|DOMWindow}