1970909 - Auto-expanding <details/> while using Find-in-page toggles <details/> weirdly

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Teun van Eijsden]

Attachment: autoexpandingdetailstest.html

User Agent: Links (2.2; Linux 2.6.32-gentoo-r6 x86_64; 129x42)

Steps to reproduce:

1. Visit a page with multiple <details/> elements.
2. Use Find-in-page (Cmd-F) and search for the letter "a" (that appears lots of times in the page)
3. Press enter repeatedly to search for the next occurrence of this letter.

Actual results:

When the search hits a result inside a <details/> element, it briefly opens that <details/> element, then closes it again and opens the very last (of many) <details/> elements on the page, which stays open

Expected results:

Only the <details/> element where the hit occurs should stay open.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Teun van Eijsden]

Please ignore the "User Agent: Links..." in the original description (can't edit it?)

Comment 3

[Stephen Thompson [:sthompson]]

This was also reported and filed to Toolkit::Find Toolbar with bug 1973904. Not sure which component needs to be assigned in order to get triaged by the right team.

Comment 6

[Jan Jaeschke [:jjaschke]]

Attachment: 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 elments
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.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ab69e32e0298
https://hg.mozilla.org/integration/autoland/rev/c4a1de8ce4a1
Call reveal algorithm for callers of find algorithm instead of the find algorithm itself. r=emilio

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/55535 for changes under testing/web-platform/tests

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/c4a1de8ce4a1

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot