tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit ac33ad716c68ded2818d7886a37a22204feb86aa
parent eeb0443b54ecb31c803b37580db3de4fc7bb39b8
Author: Nicolas Chevobbe <nchevobbe@mozilla.com>
Date:   Sun, 23 Nov 2025 22:45:19 +0000

Bug 1999997 - [devtools] Allow to add rule on pseudo elements. r=devtools-reviewers,jdescottes.

In addNewRule, we retrieve the binding element and pseudo from the passed node,
and if it is a pseudo element, we append the pseudo element type at the end of
the selector.
Tests are updated to check the new behavior.

Differential Revision: https://phabricator.services.mozilla.com/D272603

Diffstat:
Mdevtools/client/inspector/rules/rules.js | 24+++++++++++++++---------
Mdevtools/client/inspector/rules/test/browser_rules_add-rule-button-state.js | 61++++++++++++++++++++++++++++++++++++++++++++++++++-----------
Mdevtools/client/inspector/rules/test/browser_rules_add-rule-with-menu.js | 105+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
Mdevtools/client/inspector/rules/test/browser_rules_add-rule.js | 26++++++++++++++++++++++----
Mdevtools/client/inspector/shared/style-inspector-menu.js | 5++---
Mdevtools/server/actors/page-style.js | 24+++++++++++++++++++-----
6 files changed, 190 insertions(+), 55 deletions(-)

diff --git a/devtools/client/inspector/rules/rules.js b/devtools/client/inspector/rules/rules.js @@ -164,7 +164,7 @@ function CssRuleView(inspector, document, store) { this._outputParser = new OutputParser(document, this.cssProperties); this._abortController = new this.styleWindow.AbortController(); - this._onAddRule = this._onAddRule.bind(this); + this.addNewRule = this.addNewRule.bind(this); this._onContextMenu = this._onContextMenu.bind(this); this._onCopy = this._onCopy.bind(this); this._onFilterStyles = this._onFilterStyles.bind(this); @@ -226,7 +226,7 @@ function CssRuleView(inspector, document, store) { ); this.element.addEventListener("copy", this._onCopy); this.element.addEventListener("contextmenu", this._onContextMenu); - this.addRuleButton.addEventListener("click", this._onAddRule); + this.addRuleButton.addEventListener("click", this.addNewRule); this.searchField.addEventListener("input", this._onFilterStyles); this.searchClearButton.addEventListener("click", this._onClearSearch); this.pseudoClassToggle.addEventListener( @@ -752,7 +752,7 @@ CssRuleView.prototype = { /** * Add a new rule to the current element. */ - async _onAddRule() { + addNewRule() { const elementStyle = this._elementStyle; const element = elementStyle.element; const pseudoClasses = element.pseudoClassLocks; @@ -765,14 +765,20 @@ CssRuleView.prototype = { }, /** + * Returns true if the "Add Rule" action (either via the addRuleButton or the context + * menu entry) can be performed for the currently selected node. + * + * @returns {boolean} + */ + canAddNewRuleForSelectedNode() { + return this._viewedElement && this.inspector.selection.isElementNode(); + }, + + /** * Disables add rule button when needed */ refreshAddRuleButtonState() { - const shouldBeDisabled = - !this._viewedElement || - !this.inspector.selection.isElementNode() || - this.inspector.selection.isNativeAnonymousNode(); - this.addRuleButton.disabled = shouldBeDisabled; + this.addRuleButton.disabled = !this.canAddNewRuleForSelectedNode(); }, /** @@ -1008,7 +1014,7 @@ CssRuleView.prototype = { this.styleDocument.removeEventListener("click", this, { capture: true }); this.element.removeEventListener("copy", this._onCopy); this.element.removeEventListener("contextmenu", this._onContextMenu); - this.addRuleButton.removeEventListener("click", this._onAddRule); + this.addRuleButton.removeEventListener("click", this.addNewRule); this.searchField.removeEventListener("input", this._onFilterStyles); this.searchClearButton.removeEventListener("click", this._onClearSearch); this.pseudoClassPanel.removeEventListener( diff --git a/devtools/client/inspector/rules/test/browser_rules_add-rule-button-state.js b/devtools/client/inspector/rules/test/browser_rules_add-rule-button-state.js @@ -3,8 +3,9 @@ "use strict"; -// Tests if the `Add rule` button disables itself properly for non-element nodes -// and anonymous element. +const nodeConstants = require("resource://devtools/shared/dom-node-constants.js"); + +// Tests if the `Add rule` button disables itself properly for non-element nodes. const TEST_URI = ` <style type="text/css"> @@ -12,7 +13,12 @@ const TEST_URI = ` content: "before"; } </style> - <div id="pseudo"></div> + <div id="pseudo">${ + // put a text node big enough so the text node is not inlined + "pseudo ".repeat(50) + } + </div> + <-- my comment --> <div id="testid">Test Node</div> `; @@ -27,24 +33,57 @@ async function testDisabledButton(inspector, view) { info("Selecting a real element"); await selectNode(node, inspector); - ok(!view.addRuleButton.disabled, "Add rule button should be enabled"); + ok( + !view.addRuleButton.disabled, + "Add rule button should be enabled for regular element" + ); - info("Select a null element"); + info("Clear selection"); await view.selectElement(null); - ok(view.addRuleButton.disabled, "Add rule button should be disabled"); + ok( + view.addRuleButton.disabled, + "Add rule button should be disabled when no element is selected" + ); info("Selecting a real element"); await selectNode(node, inspector); - ok(!view.addRuleButton.disabled, "Add rule button should be enabled"); + ok( + !view.addRuleButton.disabled, + "Add rule button should be enabled again when selecting regular element" + ); info("Selecting a pseudo element"); const pseudo = await getNodeFront("#pseudo", inspector); const children = await inspector.walker.children(pseudo); - const before = children.nodes[0]; - await selectNode(before, inspector); - ok(view.addRuleButton.disabled, "Add rule button should be disabled"); + const [beforeNodeFront, textNodeFront] = children.nodes; + await selectNode(beforeNodeFront, inspector); + // sanity check + is( + inspector.selection.nodeFront.displayName, + "::before", + "We selected the ::before pseudo element" + ); + ok( + !view.addRuleButton.disabled, + "Add rule button should be enabled for pseudo element" + ); + + await selectNode(textNodeFront, inspector); + // sanity check + is( + inspector.selection.nodeFront.nodeType, + nodeConstants.TEXT_NODE, + "We selected the text node" + ); + ok( + view.addRuleButton.disabled, + "Add rule button should be disabled for text node" + ); info("Selecting a real element"); await selectNode(node, inspector); - ok(!view.addRuleButton.disabled, "Add rule button should be enabled"); + ok( + !view.addRuleButton.disabled, + "Add rule button should be enabled again when selecting regular element" + ); } diff --git a/devtools/client/inspector/rules/test/browser_rules_add-rule-with-menu.js b/devtools/client/inspector/rules/test/browser_rules_add-rule-with-menu.js @@ -4,41 +4,100 @@ "use strict"; // Tests the a new CSS rule can be added using the context menu. +const nodeConstants = require("resource://devtools/shared/dom-node-constants.js"); -const TEST_URI = '<div id="testid">Test Node</div>'; +const TEST_URI = ` + <style> + :where(#testid)::before { + content: "before "; + } + </style> + <div id="testid">${ + // put a text node big enough so the text node is not inlined + "Test Node ".repeat(50) + }</div>`; add_task(async function () { await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); const { inspector, view } = await openRuleView(); - await selectNode("#testid", inspector); - await addNewRuleFromContextMenu(inspector, view); - await testNewRule(view); -}); + // Retrieve the different node fronts we're going to use + const testidNodeFront = await getNodeFront("#testid", inspector); + const children = await inspector.walker.children(testidNodeFront); + const [beforeNodeFront, textNodeFront] = children.nodes; + + info("Add new rule on regular node"); + { + await selectNode(testidNodeFront, inspector); + + info("Waiting for context menu to be shown"); + const menuitemAddRule = getAddNewRuleContextMenuItem(view); + ok(menuitemAddRule.visible, "Add rule is visible"); + ok(!menuitemAddRule.disabled, "Add rule is not disabled"); + + info("Adding the new rule and expecting a new-rule-added event"); + const onNewRuleAdded = view.once("new-rule-added"); + menuitemAddRule.click(); + await onNewRuleAdded; + + const ruleEditor = getRuleViewRuleEditor(view, 4); + const editor = ruleEditor.selectorText.ownerDocument.activeElement; + is(editor.value, "#testid", "Selector editor value is as expected"); + + // Escaping from the selector field + EventUtils.synthesizeKey("KEY_Escape"); + } + + info("Add new rule on pseudo element node"); + { + await selectNode(beforeNodeFront, inspector); + // sanity check + is( + inspector.selection.nodeFront.displayName, + "::before", + "We selected the ::before pseudo element" + ); + + info("Waiting for context menu to be shown"); + const menuitemAddRule = getAddNewRuleContextMenuItem(view); + ok(menuitemAddRule.visible, "Add rule is visible"); + ok(!menuitemAddRule.disabled, "Add rule is not disabled"); + + info("Adding the new rule and expecting a new-rule-added event"); + const onNewRuleAdded = view.once("new-rule-added"); + menuitemAddRule.click(); + await onNewRuleAdded; + + const ruleEditor = getRuleViewRuleEditor(view, 0); + const editor = ruleEditor.selectorText.ownerDocument.activeElement; + is(editor.value, "#testid::before", "Selector editor value is as expected"); + + // Escaping from the selector field + EventUtils.synthesizeKey("KEY_Escape"); + } -async function addNewRuleFromContextMenu(inspector, view) { + info("Check that context menu is disabled when text node is selected"); + await selectNode(textNodeFront, inspector); + // sanity check + is( + inspector.selection.nodeFront.nodeType, + nodeConstants.TEXT_NODE, + "We selected the text node" + ); info("Waiting for context menu to be shown"); + const menuitemAddRule = getAddNewRuleContextMenuItem(view); + ok(menuitemAddRule.visible, "Add rule is visible"); + ok( + menuitemAddRule.disabled, + "Add rule is disabled when a text node is selected" + ); +}); +function getAddNewRuleContextMenuItem(view) { const allMenuItems = openStyleContextMenuAndGetAllItems(view, view.element); - const menuitemAddRule = allMenuItems.find( + return allMenuItems.find( item => item.label === STYLE_INSPECTOR_L10N.getStr("styleinspector.contextmenu.addNewRule") ); - - ok(menuitemAddRule.visible, "Add rule is visible"); - - info("Adding the new rule and expecting a new-rule-added event"); - const onNewRuleAdded = view.once("new-rule-added"); - menuitemAddRule.click(); - await onNewRuleAdded; -} - -function testNewRule(view) { - const ruleEditor = getRuleViewRuleEditor(view, 1); - const editor = ruleEditor.selectorText.ownerDocument.activeElement; - is(editor.value, "#testid", "Selector editor value is as expected"); - - info("Escaping from the selector field the change"); - EventUtils.synthesizeKey("KEY_Escape"); } diff --git a/devtools/client/inspector/rules/test/browser_rules_add-rule.js b/devtools/client/inspector/rules/test/browser_rules_add-rule.js @@ -6,9 +6,13 @@ // Tests adding a new rule using the add rule button. const TEST_URI = ` - <style type="text/css"> + <style> .testclass { text-align: center; + + &::after { + content: "-"; + } } </style> <div id="testid" class="testclass">Styled Node</div> @@ -25,7 +29,7 @@ const TEST_URI = ` `; const TEST_DATA = [ - { node: "#testid", expected: "#testid" }, + { node: "#testid", expected: "#testid", expectedIndex: 4 }, { node: ".testclass2", expected: ".testclass2" }, { node: ".class1.class2", expected: ".class1.class2" }, { node: ".class3.class4", expected: ".class3.class4" }, @@ -40,11 +44,25 @@ add_task(async function () { const { inspector, view } = await openRuleView(); for (const data of TEST_DATA) { - const { node, expected } = data; + const { node, expected, expectedIndex = 1 } = data; await selectNode(node, inspector); - await addNewRuleAndDismissEditor(inspector, view, expected, 1); + await addNewRuleAndDismissEditor(inspector, view, expected, expectedIndex); } + info("Check that we can add rule for pseudo element node"); + const testidNodeFront = await getNodeFront("#testid", inspector); + const testidNodeFrontChildren = + await inspector.walker.children(testidNodeFront); + const afterNodeFront = testidNodeFrontChildren.nodes.at(-1); + await selectNode(afterNodeFront, inspector); + // sanity check + is( + inspector.selection.nodeFront.displayName, + "::after", + "We selected the ::after pseudo element" + ); + await addNewRuleAndDismissEditor(inspector, view, "#testid::after", 0); + info(`Check that clicking the "Add Rule" button clears the filter`); await selectNode("footer", inspector); is( diff --git a/devtools/client/inspector/shared/style-inspector-menu.js b/devtools/client/inspector/shared/style-inspector-menu.js @@ -254,10 +254,9 @@ StyleInspectorMenu.prototype = { "styleinspector.contextmenu.addNewRule" ), accesskey: STYLE_INSPECTOR_L10N.getStr(addRuleAccessKey), - click: () => this.view._onAddRule(), + click: () => this.view.addNewRule(), visible: this.isRuleView, - disabled: - !this.isRuleView || this.inspector.selection.isNativeAnonymousNode(), + disabled: !this.isRuleView || !this.view.canAddNewRuleForSelectedNode(), }); menu.append(menuitemAddRule); diff --git a/devtools/server/actors/page-style.js b/devtools/server/actors/page-style.js @@ -1281,18 +1281,32 @@ class PageStyleActor extends Actor { } const cssRules = sheet.cssRules; - const rawNode = node.rawNode; - const classes = [...rawNode.classList]; + + // Get the binding element in case node is a pseudo element, so we can properly + // build the selector + const { bindingElement, pseudo } = CssLogic.getBindingElementAndPseudo( + node.rawNode + ); + const classes = [...bindingElement.classList]; let selector; - if (rawNode.id) { - selector = "#" + CSS.escape(rawNode.id); + if (bindingElement.id) { + selector = "#" + CSS.escape(bindingElement.id); } else if (classes.length) { selector = "." + classes.map(c => CSS.escape(c)).join("."); } else { - selector = rawNode.localName; + selector = bindingElement.localName; } + if (pseudo && pseudoClasses?.length) { + throw new Error( + `Can't set pseudo classes (${JSON.stringify(pseudoClasses)}) onto a pseudo element (${pseudo})` + ); + } + + if (pseudo) { + selector += pseudo; + } if (pseudoClasses && pseudoClasses.length) { selector += pseudoClasses.join(""); }