commit 4b64cad5f815ea2189bd04f02efe5068c5d64af7
parent 2c6f8ffd668e71503b6989d07a8c25748e4be29b
Author: Logan Rosen <loganrosen@gmail.com>
Date: Mon, 5 Jan 2026 11:30:50 +0000
Bug 845363 - Replace gatherTextUnder with nsIDocumentEncoder. r=Gijs
This replaces the manual tree-walking implementation of gatherTextUnder
with nsIDocumentEncoder for proper text extraction.
This also fixes bug 306937 by eliminating spurious whitespace that was
introduced around inline formatting elements.
Differential Revision: https://phabricator.services.mozilla.com/D277395
Diffstat:
4 files changed, 85 insertions(+), 75 deletions(-)
diff --git a/browser/actors/ContextMenuChild.sys.mjs b/browser/actors/ContextMenuChild.sys.mjs
@@ -401,45 +401,17 @@ export class ContextMenuChild extends JSWindowActorChild {
}
// Gather all descendent text under given document node.
+ // NOTE: Keep this in sync with gatherTextUnder in
+ // browser/base/content/utilityOverlay.js
_gatherTextUnder(root) {
- let text = "";
- let node = root.firstChild;
- let depth = 1;
- while (node && depth > 0) {
- // See if this node is text.
- if (node.nodeType == node.TEXT_NODE) {
- // Add this text to our collection.
- text += " " + node.data;
- } else if (this.contentWindow.HTMLImageElement.isInstance(node)) {
- // If it has an "alt" attribute, add that.
- let altText = node.getAttribute("alt");
- if (altText && altText != "") {
- text += " " + altText;
- }
- }
- // Find next node to test.
- // First, see if this node has children.
- if (node.hasChildNodes()) {
- // Go to first child.
- node = node.firstChild;
- depth++;
- } else {
- // No children, try next sibling (or parent next sibling).
- while (depth > 0 && !node.nextSibling) {
- node = node.parentNode;
- depth--;
- }
- if (node.nextSibling) {
- node = node.nextSibling;
- }
- }
- }
-
- // Strip leading and tailing whitespace.
- text = text.trim();
- // Compress remaining whitespace.
- text = text.replace(/\s+/g, " ");
- return text;
+ const encoder = Cu.createDocumentEncoder("text/plain");
+ encoder.init(
+ root.ownerDocument,
+ "text/plain",
+ Ci.nsIDocumentEncoder.SkipInvisibleContent
+ );
+ encoder.setNode(root);
+ return encoder.encodeToString().trim();
}
// Returns a "url"-type computed style attribute value, with the url() stripped.
diff --git a/browser/base/content/test/contextMenu/browser.toml b/browser/base/content/test/contextMenu/browser.toml
@@ -41,6 +41,8 @@ skip-if = [
"os == 'win'", # Bug 1719856
]
+["browser_contextmenu_bookmark_link_text.js"]
+
["browser_contextmenu_contenteditable.js"]
["browser_contextmenu_cross_boundary_selection.js"]
diff --git a/browser/base/content/test/contextMenu/browser_contextmenu_bookmark_link_text.js b/browser/base/content/test/contextMenu/browser_contextmenu_bookmark_link_text.js
@@ -0,0 +1,63 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Test for Bug 306937 - "Bookmark This Link..." context menu should not
+ * add spurious whitespace characters to bookmark names when links contain
+ * inline HTML formatting elements.
+ */
+
+add_task(async function test_bookmark_link_text_with_inline_formatting() {
+ const TEST_PAGE = `data:text/html,
+ <a id="link1" href="https://example.com/1">Co<u>m</u>ponent</a>
+ <a id="link2" href="https://example.com/2">@<span>username</span></a>
+ <a id="link3" href="https://example.com/3"><span>Multiple</span> <span>Spans</span></a>
+ <a id="link4" href="https://example.com/4">Text <strong>bold</strong> more</a>
+ <a id="link5" href="https://example.com/5"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==" alt="Icon"> Text</a>
+ <a id="link6" href="https://example.com/6">Normal <em>text</em> here</a>
+ <a id="link7" href="https://example.com/7">Text with multiple spaces</a>
+ `;
+
+ await BrowserTestUtils.withNewTab(TEST_PAGE, async browser => {
+ const tests = [
+ { selector: "#link1", expected: "Component" },
+ { selector: "#link2", expected: "@username" },
+ { selector: "#link3", expected: "Multiple Spans" },
+ { selector: "#link4", expected: "Text bold more" },
+ { selector: "#link5", expected: "Icon Text" },
+ { selector: "#link6", expected: "Normal text here" },
+ { selector: "#link7", expected: "Text with multiple spaces" },
+ ];
+
+ for (const test of tests) {
+ let contextMenu = document.getElementById("contentAreaContextMenu");
+ let popupShown = BrowserTestUtils.waitForEvent(contextMenu, "popupshown");
+
+ BrowserTestUtils.synthesizeMouseAtCenter(
+ test.selector,
+ { type: "contextmenu", button: 2 },
+ browser
+ );
+
+ await popupShown;
+
+ // Get the linkText from the context menu's context
+ let linkText = gContextMenu.linkTextStr;
+
+ is(
+ linkText,
+ test.expected,
+ `Link text for ${test.selector} should be "${test.expected}" without spurious spaces`
+ );
+
+ let popupHidden = BrowserTestUtils.waitForEvent(
+ contextMenu,
+ "popuphidden"
+ );
+ contextMenu.hidePopup();
+ await popupHidden;
+ }
+ });
+});
diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js
@@ -325,44 +325,17 @@ function eventMatchesKey(aEvent, aKey) {
}
// Gather all descendent text under given document node.
+// NOTE: Keep this in sync with _gatherTextUnder in
+// browser/actors/ContextMenuChild.sys.mjs
function gatherTextUnder(root) {
- var text = "";
- var node = root.firstChild;
- var depth = 1;
- while (node && depth > 0) {
- // See if this node is text.
- if (node.nodeType == Node.TEXT_NODE) {
- // Add this text to our collection.
- text += " " + node.data;
- } else if (HTMLImageElement.isInstance(node)) {
- // If it has an "alt" attribute, add that.
- var altText = node.getAttribute("alt");
- if (altText) {
- text += " " + altText;
- }
- }
- // Find next node to test.
- // First, see if this node has children.
- if (node.hasChildNodes()) {
- // Go to first child.
- node = node.firstChild;
- depth++;
- } else {
- // No children, try next sibling (or parent next sibling).
- while (depth > 0 && !node.nextSibling) {
- node = node.parentNode;
- depth--;
- }
- if (node.nextSibling) {
- node = node.nextSibling;
- }
- }
- }
- // Strip leading and tailing whitespace.
- text = text.trim();
- // Compress remaining whitespace.
- text = text.replace(/\s+/g, " ");
- return text;
+ const encoder = Cu.createDocumentEncoder("text/plain");
+ encoder.init(
+ root.ownerDocument,
+ "text/plain",
+ Ci.nsIDocumentEncoder.SkipInvisibleContent
+ );
+ encoder.setNode(root);
+ return encoder.encodeToString().trim();
}
// This function exists for legacy reasons.
