commit ab69e32e0298c7d364622be19d65a8b85a182264
parent 485624dbe95b53a760b77ac56ea95327cd9977c7
Author: Jan-Niklas Jaeschke <jjaschke@mozilla.com>
Date: Sun, 19 Oct 2025 17:08:10 +0000
Bug 1970909: Call reveal algorithm for callers of find algorithm instead of the find algorithm itself. r=emilio
The previous behavior caused all sorts of edge-casey issues:
Find-in-page would open all <details> or hidden=until-found elements
which contain the search string. Therefore, accordions would open
one after another, and the last one would be opened instead of the one
with the match.
Finding a text fragment on a page would open all details/hidden=until-found elements
which contain any of a context term of the text directive.
Creating a text fragment link (context menu -> Copy Link to Highlight) would open
all hidden=until-found/details elements which contain the target range.
The reason for this behavior was `nsFind::Find()` calling the reveal algorithm unconditionally.
This patch fixes this by only calling the reveal algorithm at the appropriate places,
which is inside the find-in-page backend (`nsTypeAheadFind.cpp`),
in the `window.find()` backend (`nsWebBrowserFind.cpp`),
and in the text fragment finder backend (`TextDirectiveFinder.cpp`).
In addition, this patch refactors the tests for finding hidden=until-found / <details> elements:
- test_nsFind.html only tests that text inside hidden=until-found or <details> can be found
- New tests in `browser_findbar_hidden_beforematch.js` and `test_find.html` ensure that
the elements are visible after they are found, and that `beforematch` is fired for hidden=until-found elements
- New tests have been added for finding and creating text fragments to ensure that only the correct elements are revealed.
Differential Revision: https://phabricator.services.mozilla.com/D269160
Diffstat:
13 files changed, 414 insertions(+), 38 deletions(-)
diff --git a/dom/base/TextDirectiveFinder.cpp b/dom/base/TextDirectiveFinder.cpp
@@ -77,6 +77,9 @@ nsTArray<RefPtr<nsRange>> TextDirectiveFinder::FindTextDirectivesInDocument() {
textDirectiveRanges.AppendElement(range);
TEXT_FRAGMENT_LOG("Found text directive '{}'",
ToString(textDirective).c_str());
+ if (RefPtr startNode = range->GetStartContainer()) {
+ startNode->QueueAncestorRevealingAlgorithm();
+ }
} else {
uninvokedTextDirectives.AppendElement(std::move(textDirective));
}
diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp
@@ -4181,6 +4181,14 @@ ShadowRoot* nsINode::GetShadowRootForSelection() const {
return shadowRoot;
}
+void nsINode::QueueAncestorRevealingAlgorithm() {
+ NS_DispatchToMainThread(NS_NewRunnableFunction(
+ "RevealAncestors",
+ [self = RefPtr{this}]() MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
+ self->AncestorRevealingAlgorithm(IgnoreErrors());
+ }));
+}
+
enum class RevealType : uint8_t {
UntilFound,
Details,
diff --git a/dom/base/nsINode.h b/dom/base/nsINode.h
@@ -2546,6 +2546,8 @@ class nsINode : public mozilla::dom::EventTarget {
return HasSlots() ? GetExistingSlots()->mWeakReference : nullptr;
}
+ void QueueAncestorRevealingAlgorithm();
+
MOZ_CAN_RUN_SCRIPT void AncestorRevealingAlgorithm(ErrorResult& aRv);
protected:
diff --git a/dom/base/test/mochitest.toml b/dom/base/test/mochitest.toml
@@ -1696,6 +1696,8 @@ skip-if = [
"os == 'android'", # The test needs to run reasonably fast.
]
+["test_text-fragments-create-text-directive-no-reveal.html"]
+
["test_text-fragments-create-text-directive.html"]
["test_text-fragments-get-text-directive-ranges.html"]
diff --git a/dom/base/test/test_find.html b/dom/base/test/test_find.html
@@ -198,6 +198,69 @@ let runTests = t.step_func_done(function() {
testFindable(false, "find me", `
Do you find <span inert>not findable</span> me?
`, "boundary-crossing inert");
+
+ testFindable(true, "hidden content", `
+ <div hidden="until-found">This is hidden content</div>
+ `, "hidden=until-found should be findable");
+
+ testFindable(true, "details content", `
+ <details>
+ <summary>Click to expand</summary>
+ <div>This is details content</div>
+ </details>
+ `, "closed details should be findable");
+
+ promise_test(async function() {
+ const iframe = document.querySelector("iframe");
+ iframe.contentDocument.documentElement.innerHTML = `
+ <div id="hidden" hidden="until-found">findable text</div>
+ `;
+
+ const beforematchPromise = new Promise(resolve => {
+ iframe.contentDocument.getElementById("hidden").addEventListener("beforematch", () => {
+ resolve();
+ }, { once: true });
+ });
+
+ iframe.contentWindow.getSelection().removeAllRanges();
+ const found = iframe.contentWindow.find("findable");
+
+ assert_true(found, "Should find text in hidden=until-found");
+
+ await beforematchPromise;
+
+ await new Promise(resolve => iframe.contentWindow.requestAnimationFrame(resolve));
+
+ const hiddenElement = iframe.contentDocument.getElementById("hidden");
+ assert_equals(hiddenElement.hidden, false, "hidden attribute should be removed");
+ }, "beforematch event fires and element is revealed for hidden=until-found");
+
+ testFindable(false, "should not be found", `
+ <style>#outer { content-visibility: hidden; }</style>
+ <div id="outer">
+ <div hidden="until-found">This should not be found</div>
+ </div>
+ `, "hidden=until-found inside content-visibility:hidden should not be findable");
+
+ promise_test(async function() {
+ const iframe = document.querySelector("iframe");
+ iframe.contentDocument.documentElement.innerHTML = `
+ <details id="details">
+ <summary>Summary</summary>
+ <div>Details text</div>
+ </details>
+ `;
+
+ iframe.contentWindow.getSelection().removeAllRanges();
+ const found = iframe.contentWindow.find("Details text");
+
+ assert_true(found, "Should find text in closed details");
+
+ await new Promise(resolve => iframe.contentWindow.requestAnimationFrame(resolve));
+
+ const detailsElement = iframe.contentDocument.getElementById("details");
+ assert_true(detailsElement.open, "details element should be opened after find");
+ }, "details element is opened after find");
});
window.onload = function() {
diff --git a/dom/base/test/test_text-fragments-create-text-directive-no-reveal.html b/dom/base/test/test_text-fragments-create-text-directive-no-reveal.html
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>Text Fragment Creation Should Not Reveal Other Elements (Bug 1970909)</title>
+ <meta charset="UTF-8">
+ <link rel="help" href="https://bugzil.la/1970909">
+ <script src="/tests/SimpleTest/SimpleTest.js"></script>
+</head>
+<body>
+ <div id="hidden1" hidden="until-found">abc</div>
+
+ <details id="details1">
+ <summary>First Section</summary>
+ <div id="content1">abc</div>
+ </details>
+
+ <details id="details2">
+ <summary>Second Section</summary>
+ <div id="content2">abc def</div>
+ </details>
+
+ <script>
+ SimpleTest.waitForExplicitFinish();
+
+ async function runTests() {
+ try {
+ await SpecialPowers.pushPrefEnv({"set": [
+ ["dom.text_fragments.enabled", true],
+ ["dom.text_fragments.create_text_fragment.enabled", true],
+ ]});
+
+ // Creating a text fragment from a selection in details2 should not reveal
+ // other elements (closed <details> or hidden=until-found elements).
+
+ ok(hidden1.hidden === "until-found",
+ "Precondition: hidden1 should have hidden=until-found");
+ details1.open = false;
+ details2.open = true;
+
+ ok(!details1.open, "Precondition: details1 should be closed");
+ ok(details2.open, "Precondition: details2 should be open");
+
+ const range = document.createRange();
+ range.setStart(content2.firstChild, 0);
+ range.setEnd(content2.firstChild, 3);
+ is(range.toString(), "abc", "Precondition: Range should contain 'abc'");
+
+ // Create text directive from the range
+ // This will internally search for "abc" to determine if context is needed
+ // and must not reveal other matches of the search string.
+ const textDirective = await SpecialPowers.wrap(document).fragmentDirective.createTextDirectiveForRanges([range]);
+
+ await new Promise(resolve => requestAnimationFrame(resolve));
+
+ isnot(textDirective, null, "A text directive should be created for 'abc'");
+
+ is(hidden1.hidden, "until-found", "hidden=until-found element should not be revealed");
+ ok(!details1.open, "closed details element should not be opened");
+ ok(details2.open, "already open element which contains the selection should remain open");
+ is(textDirective, "text=abc,-def", `Text directive "${textDirective}" should be correct`);
+
+ } finally {
+ SimpleTest.finish();
+ }
+ }
+
+ document.body.onload = runTests;
+ </script>
+</body>
+</html>
diff --git a/testing/web-platform/tests/scroll-to-text-fragment/find-range-from-text-directive-no-reveal.html b/testing/web-platform/tests/scroll-to-text-fragment/find-range-from-text-directive-no-reveal.html
@@ -0,0 +1,36 @@
+<!doctype html>
+<title>Finding the text directive range must not reveal other hidden elements</title>
+<meta charset=utf-8>
+<link rel="help" href="https://bugzil.la/1970909">
+<link rel="help" href="https://wicg.github.io/ScrollToTextFragment/">
+<link rel="author" title="Jan-Niklas Jaeschke" href="mailto:jjaschke@mozilla.com">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+
+<div id="hidden1" hidden="until-found">abc</div>
+<details id="details1">
+ <summary>First Section</summary>
+ <div>abc</div>
+</details>
+<details id="details2">
+ <summary>Second Section</summary>
+ <div>abc def</div>
+</details>
+
+<script>
+// Test that navigating to a text fragment does not open hidden=until-found or
+// closed <details> elements that contain an invalid match for the text fragment.
+promise_test(async t => {
+
+ window.location.hash = ':~:text=abc,-def';
+
+ await new Promise(resolve => requestAnimationFrame(resolve));
+
+ assert_equals(hidden1.hidden, 'until-found', 'hidden=until-found element should remain hidden');
+ assert_false(details1.open, 'closed <details> element should remain closed');
+ assert_true(details2.open, '<details> element containing the match should be opened');
+}, 'Text fragment with suffix should only reveal the matching details element');
+</script>
+</body>
+</html>
diff --git a/toolkit/components/find/nsFind.cpp b/toolkit/components/find/nsFind.cpp
@@ -1010,13 +1010,6 @@ already_AddRefed<nsRange> nsFind::FindFromRangeBoundaries(
if (!rv.Failed()) {
range->SetEnd(*endParent, matchEndOffset, rv);
}
- // https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found
- NS_DispatchToMainThread(NS_NewRunnableFunction(
- "RevealHiddenUntilFound",
- [node = RefPtr(startParent)]()
- MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
- node->AncestorRevealingAlgorithm(IgnoreErrors());
- }));
if (!rv.Failed()) {
return range.forget();
}
diff --git a/toolkit/components/find/nsWebBrowserFind.cpp b/toolkit/components/find/nsWebBrowserFind.cpp
@@ -634,6 +634,11 @@ nsresult nsWebBrowserFind::SearchInFrame(nsPIDOMWindowOuter* aWindow,
if (NS_SUCCEEDED(rv) && foundRange) {
*aDidFind = true;
+ // Reveal hidden-until-found and closed details elements for the match.
+ // https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found
+ if (RefPtr startNode = foundRange->GetStartContainer()) {
+ startNode->QueueAncestorRevealingAlgorithm();
+ }
sel->RemoveAllRanges(IgnoreErrors());
// Beware! This may flush notifications via synchronous
// ScrollSelectionIntoView.
diff --git a/toolkit/components/find/test/mochitest/test_nsFind.html b/toolkit/components/find/test/mochitest/test_nsFind.html
@@ -278,42 +278,15 @@ async function runTests() {
assertNotFound(quotes, "Tardigrade");
assertNotFound(quotes, "Amoeba");
- // hidden=until-found content is found
+ // nsFind should be able to find text inside hidden=until-found
+ // or closed <details> elements.
+ // Note: nsFind itself does not trigger the reveal algorithm; that is the
+ // responsibility of the caller (e.g., find-in-page, nsTypeAheadFind).
// https://html.spec.whatwg.org/#attr-hidden-until-found
- const hiddenUntilFound = document.querySelector("#hidden-until-found");
- // Create a promise that resolves when the beforematch event fires.
- const beforematchPromise = new Promise(resolve => {
- hiddenUntilFound.onbeforematch = () => resolve();
- });
- is(
- getComputedStyle(hiddenUntilFound).contentVisibility,
- "hidden",
- "hidden=until-found content has content-visibility: hidden before being found"
- );
assertFound(document.body, "hidden until found");
- await beforematchPromise;
- await new Promise(resolve => requestAnimationFrame(resolve));
-
- is(
- getComputedStyle(hiddenUntilFound).contentVisibility,
- "visible",
- "hidden=until-found content has content-visibility: visible after being found"
- );
-
- // https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found
- const hiddenHiddenUntilFound = document.querySelector("div#hidden-until-found-inside-hidden");
- hiddenHiddenUntilFound.onbeforematch = () => ok(
- false,
- "Beforematch must not be fired for a hidden=until-found element which is inside a content-visibility:hidden element."
- );
assertNotFound(document.body, "hidden because of content-visibility: hidden outside of hidden=until-found");
- const nestedHiddenInHiddenUntilFound = document.querySelector("div#hidden-until-found-with-inner-hidden");
- nestedHiddenInHiddenUntilFound.onbeforematch = () => ok(
- false,
- "Beforematch must not be fired for a hidden=until-found element if the matched content is content-visibility:hidden"
- );
assertNotFound(document.body, "nested hidden block should not be found");
assertNotFound(document.body, "nested visibility:hidden should not be found");
diff --git a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -503,6 +503,12 @@ nsresult nsTypeAheadFind::FindItNow(uint32_t aMode, bool aIsLinksOnly,
}
mSelectionController = do_GetWeakReference(selectionController);
+ // Reveal hidden-until-found and closed details elements for the match.
+ // https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found
+ if (RefPtr startNode = returnRange->GetStartContainer()) {
+ startNode->QueueAncestorRevealingAlgorithm();
+ }
+
// Select the found text
if (selection) {
selection->RemoveAllRanges(IgnoreErrors());
diff --git a/toolkit/content/tests/browser/browser.toml b/toolkit/content/tests/browser/browser.toml
@@ -140,6 +140,8 @@ tags = "audiochannel"
["browser_findbar_disabled_manual.js"]
+["browser_findbar_hidden_beforematch.js"]
+
["browser_findbar_hiddenframes.js"]
["browser_findbar_marks.js"]
diff --git a/toolkit/content/tests/browser/browser_findbar_hidden_beforematch.js b/toolkit/content/tests/browser/browser_findbar_hidden_beforematch.js
@@ -0,0 +1,213 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// These tests verify that find-in-page properly reveals hidden
+// content (hidden=until-found and closed <details> elements) by triggering
+// the ancestor revealing algorithm and firing beforematch events.
+
+const TEST_PAGE_URI =
+ "data:text/html;charset=utf-8," +
+ encodeURIComponent(`
+<!DOCTYPE html>
+<body>
+<div id="hidden1" hidden="until-found">First hidden text</div>
+<div id="hidden2" hidden="until-found">Second hidden text</div>
+<details id="details1">
+ <summary>First Details</summary>
+ <div>First details text</div>
+</details>
+<details id="details2">
+ <summary>Second Details</summary>
+ <div>Second details text</div>
+</details>
+</body>
+`);
+
+// Helper function to wait for Find Again to complete
+async function promiseFindAgain(findbar) {
+ return new Promise(resolve => {
+ let resultListener = {
+ onFindResult() {
+ findbar.browser.finder.removeResultListener(resultListener);
+ resolve();
+ },
+ onCurrentSelection() {},
+ onMatchesCountResult() {},
+ onHighlightFinished() {},
+ };
+ findbar.browser.finder.addResultListener(resultListener);
+ findbar.onFindAgainCommand();
+ });
+}
+
+add_task(async function test_hidden_until_found_reveal_behavior() {
+ let tab = await BrowserTestUtils.openNewForegroundTab(
+ gBrowser,
+ TEST_PAGE_URI
+ );
+
+ let findbar = await gBrowser.getFindBar();
+
+ let matchCountPromise = new Promise(resolve => {
+ let resultListener = {
+ onFindResult() {},
+ onCurrentSelection() {},
+ onHighlightFinished() {},
+ onMatchesCountResult(response) {
+ if (response.total > 0) {
+ findbar.browser.finder.removeResultListener(resultListener);
+ resolve(response);
+ }
+ },
+ };
+ findbar.browser.finder.addResultListener(resultListener);
+ });
+
+ // Perform the first find with highlighting enabled to trigger match counting
+ await promiseFindFinished(gBrowser, "hidden text", true);
+
+ let matchCount = await matchCountPromise;
+ is(matchCount.total, 2, "Should find 2 matches in hidden elements");
+
+ await SpecialPowers.spawn(tab.linkedBrowser, [], () => {
+ return new Promise(resolve => content.requestAnimationFrame(resolve));
+ });
+
+ let { hidden1, hidden2 } = await SpecialPowers.spawn(
+ tab.linkedBrowser,
+ [],
+ () => {
+ return {
+ hidden1: content.document.getElementById("hidden1").hidden,
+ hidden2: content.document.getElementById("hidden2").hidden,
+ };
+ }
+ );
+
+ is(hidden1, false, "first hidden=until-found element should be revealed");
+ is(
+ hidden2,
+ "until-found",
+ "second hidden=until-found element should NOT be revealed yet"
+ );
+
+ // Set up beforematch listener for the second element
+ let secondBeforematchPromise = SpecialPowers.spawn(
+ tab.linkedBrowser,
+ [],
+ () => {
+ return new Promise(resolve => {
+ let element = content.document.getElementById("hidden2");
+ element.addEventListener("beforematch", () => resolve(), {
+ once: true,
+ });
+ });
+ }
+ );
+
+ await promiseFindAgain(findbar);
+
+ await secondBeforematchPromise;
+
+ await SpecialPowers.spawn(tab.linkedBrowser, [], () => {
+ return new Promise(resolve => content.requestAnimationFrame(resolve));
+ });
+
+ let { secondFindhidden1, secondFindhidden2 } = await SpecialPowers.spawn(
+ tab.linkedBrowser,
+ [],
+ () => {
+ return {
+ secondFindhidden1: content.document.getElementById("hidden1").hidden,
+ secondFindhidden2: content.document.getElementById("hidden2").hidden,
+ };
+ }
+ );
+ is(
+ secondFindhidden1,
+ false,
+ "first element should remain revealed after Find Next"
+ );
+ is(
+ secondFindhidden2,
+ false,
+ "Second element should be revealed after Find Next"
+ );
+
+ gBrowser.removeTab(tab);
+});
+
+add_task(async function test_details_reveal_behavior() {
+ let tab = await BrowserTestUtils.openNewForegroundTab(
+ gBrowser,
+ TEST_PAGE_URI
+ );
+
+ let findbar = await gBrowser.getFindBar();
+
+ let matchCountPromise = new Promise(resolve => {
+ let resultListener = {
+ onFindResult() {},
+ onCurrentSelection() {},
+ onHighlightFinished() {},
+ onMatchesCountResult(response) {
+ if (response.total > 0) {
+ findbar.browser.finder.removeResultListener(resultListener);
+ resolve(response);
+ }
+ },
+ };
+ findbar.browser.finder.addResultListener(resultListener);
+ });
+
+ // Perform the first find with highlighting enabled to trigger match counting
+ await promiseFindFinished(gBrowser, "details text", true);
+
+ let matchCount = await matchCountPromise;
+ is(matchCount.total, 2, "Should find 2 matches in details elements");
+
+ await SpecialPowers.spawn(tab.linkedBrowser, [], () => {
+ return new Promise(resolve => content.requestAnimationFrame(resolve));
+ });
+
+ let { open1, open2 } = await SpecialPowers.spawn(
+ tab.linkedBrowser,
+ [],
+ () => {
+ return {
+ open1: content.document.getElementById("details1").open,
+ open2: content.document.getElementById("details2").open,
+ };
+ }
+ );
+
+ ok(open1, "first <details> element should be open");
+ ok(!open2, "second <details> element should NOT be open yet");
+
+ await promiseFindAgain(findbar);
+
+ await SpecialPowers.spawn(tab.linkedBrowser, [], () => {
+ return new Promise(resolve => content.requestAnimationFrame(resolve));
+ });
+
+ let { secondFindOpen1, secondFindOpen2 } = await SpecialPowers.spawn(
+ tab.linkedBrowser,
+ [],
+ () => {
+ return {
+ secondFindOpen1: content.document.getElementById("details1").open,
+ secondFindOpen2: content.document.getElementById("details2").open,
+ };
+ }
+ );
+ ok(
+ secondFindOpen1,
+ "First details element should remain opened after Find Next"
+ );
+ ok(
+ secondFindOpen2,
+ "Second details element should be opened after Find Next"
+ );
+
+ gBrowser.removeTab(tab);
+});
