commit b391bb8e0f50c50f8d5c8712809d2102e8328cc8
parent 025f8d5456bad832f04329a37f16480f94170fff
Author: Jan-Niklas Jaeschke <jjaschke@mozilla.com>
Date: Fri, 19 Dec 2025 09:07:32 +0000
Bug 2006040, part 1: Scroll asynchronously after revealing hidden content in find-in-page. r=emilio
Before this patch, the ancestor reveal algorithm would run asynchronously,
and therefore after the synchronous scroll attempt.
This breaks find-in-page for hidden=until-found / details elements.
This patch moves the scroll logic to run after the reveal has completed.
Differential Revision: https://phabricator.services.mozilla.com/D276834
Diffstat:
3 files changed, 187 insertions(+), 17 deletions(-)
diff --git a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -503,12 +503,6 @@ 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());
@@ -524,21 +518,34 @@ nsresult nsTypeAheadFind::FindItNow(uint32_t aMode, bool aIsLinksOnly,
getter_AddRefs(mFoundLink));
}
- // Change selection color to ATTENTION and scroll to it. Careful: we
- // must wait until after we goof with focus above before changing to
- // ATTENTION, or when we MoveFocus() and the selection is not on a
- // link, we'll blur, which will lose the ATTENTION.
+ // Change selection color to ATTENTION. Careful: we must wait until
+ // after we goof with focus above before changing to ATTENTION, or when
+ // we MoveFocus() and the selection is not on a link, we'll blur, which
+ // will lose the ATTENTION.
if (selectionController) {
- // Beware! This may flush notifications via synchronous
- // ScrollSelectionIntoView.
SetSelectionModeAndRepaint(nsISelectionController::SELECTION_ATTENTION);
- selectionController->ScrollSelectionIntoView(
- SelectionType::eNormal,
- nsISelectionController::SELECTION_WHOLE_SELECTION,
- ScrollAxis(WhereToScroll::Center), ScrollAxis(), ScrollFlags::None,
- SelectionScrollMode::SyncFlush);
}
+ NS_DispatchToMainThread(NS_NewRunnableFunction(
+ "AncestorRevealingAlgorithm",
+ [self = RefPtr{this}, returnRange = RefPtr{returnRange},
+ selectionController = nsCOMPtr{selectionController}]()
+ MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA {
+ // Reveal hidden-until-found and closed details elements.
+ // https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found
+ if (RefPtr startNode = returnRange->GetStartContainer()) {
+ startNode->AncestorRevealingAlgorithm(IgnoreErrors());
+ }
+ // Scroll to the selection.
+ if (selectionController) {
+ selectionController->ScrollSelectionIntoView(
+ SelectionType::eNormal,
+ nsISelectionController::SELECTION_WHOLE_SELECTION,
+ ScrollAxis(WhereToScroll::Center), ScrollAxis(),
+ ScrollFlags::None, SelectionScrollMode::SyncFlush);
+ }
+ }));
+
SetCurrentWindow(window);
*aResult = hasWrapped ? FIND_WRAPPED : FIND_FOUND;
return NS_OK;
diff --git a/toolkit/content/tests/browser/browser.toml b/toolkit/content/tests/browser/browser.toml
@@ -157,6 +157,8 @@ skip-if = [
"os == 'win' && os_version == '11.26100' && arch == 'x86_64' && debug", # Bug 1995211
]
+["browser_findbar_hidden_reveal.js"]
+
["browser_findbar_hiddenframes.js"]
["browser_findbar_marks.js"]
diff --git a/toolkit/content/tests/browser/browser_findbar_hidden_reveal.js b/toolkit/content/tests/browser/browser_findbar_hidden_reveal.js
@@ -0,0 +1,161 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Tests that find-in-page properly reveals and scrolls to content inside
+ * hidden=until-found and closed <details> elements.
+ */
+
+add_task(async function test_findbar_reveal_hidden_until_found() {
+ const TEST_PAGE = `data:text/html,
+ <!DOCTYPE html>
+ <html>
+ <head>
+ <meta charset="utf-8">
+ <style>
+ body { margin: 0; }
+ .spacer { height: 200vh; }
+ </style>
+ </head>
+ <body>
+ <div class="spacer">Top content</div>
+ <div hidden="until-found">
+ <p id="hidden-target">SearchableHiddenText</p>
+ </div>
+ <div class="spacer">Bottom content</div>
+ </body>
+ </html>`;
+
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE);
+ let browser = tab.linkedBrowser;
+
+ // Verify initial state: element is hidden and page is not scrolled
+ let initialState = await SpecialPowers.spawn(browser, [], () => {
+ let hiddenDiv = content.document.querySelector('[hidden="until-found"]');
+ let target = content.document.getElementById("hidden-target");
+ return {
+ isHidden: hiddenDiv.hidden,
+ scrollY: content.scrollY,
+ targetVisible: target.checkVisibility(),
+ };
+ });
+
+ ok(initialState.isHidden, "Element should initially be hidden");
+ is(initialState.scrollY, 0, "Page should not be scrolled initially");
+ ok(!initialState.targetVisible, "Target should not be visible initially");
+
+ // Set up event listener for the beforematch event
+ let beforematchPromise = SpecialPowers.spawn(browser, [], () => {
+ return new Promise(resolve => {
+ let hiddenDiv = content.document.querySelector('[hidden="until-found"]');
+ hiddenDiv.addEventListener("beforematch", () => resolve(), {
+ once: true,
+ });
+ });
+ });
+
+ // Open findbar and search for text
+ await promiseFindFinished(gBrowser, "SearchableHiddenText", false);
+
+ // Wait for the reveal to complete
+ await beforematchPromise;
+
+ // Wait one more frame for scroll to complete
+ await new Promise(resolve => requestAnimationFrame(resolve));
+
+ // Verify element was revealed and page scrolled
+ let finalState = await SpecialPowers.spawn(browser, [], () => {
+ let target = content.document.getElementById("hidden-target");
+ // After revealing, the hidden attribute is removed entirely
+ let parent = target.parentElement;
+ return {
+ hasHiddenAttr: parent.hasAttribute("hidden"),
+ scrollY: content.scrollY,
+ targetVisible: target.checkVisibility(),
+ };
+ });
+
+ ok(
+ !finalState.hasHiddenAttr,
+ "Hidden attribute should be removed after find"
+ );
+ Assert.greater(finalState.scrollY, 0, "Page should be scrolled after find");
+ ok(finalState.targetVisible, "Target should be visible after find");
+
+ await BrowserTestUtils.removeTab(tab);
+});
+
+add_task(async function test_findbar_reveal_closed_details() {
+ const TEST_PAGE = `data:text/html,
+ <!DOCTYPE html>
+ <html>
+ <head>
+ <meta charset="utf-8">
+ <style>
+ body { margin: 0; }
+ .spacer { height: 200vh; }
+ </style>
+ </head>
+ <body>
+ <div class="spacer">Top content</div>
+ <details id="details-target">
+ <summary>Click to expand</summary>
+ <p id="details-content">SearchableDetailsText</p>
+ </details>
+ <div class="spacer">Bottom content</div>
+ </body>
+ </html>`;
+
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE);
+ let browser = tab.linkedBrowser;
+
+ // Verify initial state: details is closed and page is not scrolled
+ let initialState = await SpecialPowers.spawn(browser, [], () => {
+ let details = content.document.getElementById("details-target");
+ let target = content.document.getElementById("details-content");
+ return {
+ isOpen: details.open,
+ scrollY: content.scrollY,
+ targetVisible: target.checkVisibility(),
+ };
+ });
+
+ ok(!initialState.isOpen, "Details should initially be closed");
+ is(initialState.scrollY, 0, "Page should not be scrolled initially");
+ ok(!initialState.targetVisible, "Target should not be visible initially");
+
+ // Set up event listener for the toggle event
+ let togglePromise = SpecialPowers.spawn(browser, [], () => {
+ return new Promise(resolve => {
+ let details = content.document.getElementById("details-target");
+ details.addEventListener("toggle", () => resolve(), { once: true });
+ });
+ });
+
+ // Open findbar and search for text
+ await promiseFindFinished(gBrowser, "SearchableDetailsText", false);
+
+ // Wait for the reveal to complete
+ await togglePromise;
+
+ // Wait one more frame for scroll to complete
+ await new Promise(resolve => requestAnimationFrame(resolve));
+
+ // Verify details was opened and page scrolled
+ let finalState = await SpecialPowers.spawn(browser, [], () => {
+ let details = content.document.getElementById("details-target");
+ let target = content.document.getElementById("details-content");
+ return {
+ isOpen: details.open,
+ scrollY: content.scrollY,
+ targetVisible: target.checkVisibility(),
+ };
+ });
+
+ ok(finalState.isOpen, "Details should be opened after find");
+ Assert.greater(finalState.scrollY, 0, "Page should be scrolled after find");
+ ok(finalState.targetVisible, "Target should be visible after find");
+
+ await BrowserTestUtils.removeTab(tab);
+});
