Doing find-in-page for text in GitHub PR/issue pages doesn’t cause expected auto-expansion of closed “details” elements that have matches
Describe the bug
Unexpectedly, doing find-in-page in current Chrome for a given string in GitHub PR/issue pages that contain comments with details elements doesn’t cause auto-expansion of any closed details elements that have matches for the given string.
To Reproduce Steps to reproduce the behavior:
- Go to https://github.com/whatwg/html/pull/9778 in current Chrome.
- Do a find-in-page for the string “web service” — which occurs multiple times in the contents of the More
detailselement in the issue description. - Notice that no match is found.
- Open the More
detailselement in the issue description (by clicking the disclosure triangle). - Do a find-in-page for the string “web service” with that More
detailselement open. - Notice that multiple matches are found .
Expected behavior
With current Chrome, when doing find-in-page in GitHub PR/issue descriptions for some particular string , it’s expected that any closed details elements whose contents have matches for that string will auto-expand to reveal the matches.
That’s the expected behavior because it’s what’s required in the current HTML spec:
When find-in-page begins searching for matches, all
detailselements in the page which do not have theiropenattribute set should have the skipped contents of their second slot become accessible, without modifying theopenattribute, in order to make find-in-page able to search through it.
…and that’s what current Chrome supports — and what Safari will also have support for in the near future.
Desktop (please complete the following information):
- OS: macOS
- Browser: Chrome
- Version: 123
Additional context The cause appears to be this: https://github.com/primer/css/blob/8673252106fbbfec1e640e56fc5c8b13a977f509/src/base/base.scss#L95-L106
That is, specifically, the styling that sets display: none !important on details contents.
I can understand why that was added at the time (~6 years ago) — but the details element is now well-supported in browsers, so that styling no longer seems useful.
But anyway, that styling as-is now is preventing users from being able to benefit from the details-auto-expand feature when searching for strings in details elements in GitHub PRs and issues — which seems not great, since people (and bots) are using details in GitHub issue/PR comments quite a lot.
Note also: work is in progress on adding the details-auto-expand feature to Safari in the very near future. So we’ll soon have support in two major browsers to let users find text in closed details contents in GitHub PRs and issues — but it’ll only work if the primer styling for contents of closed details is updated to not have display: none !important.
@sideshowbarker Thanks for taking the time to open an issue! ❤
@langermank, Does this look safe to delete? I don't think we support any browsers that don't automatically handle this anymore (https://caniuse.com/mdn-html_elements_details_open)
&:not([open]) {
// Set details content hidden by default for browsers that don't do this
> *:not(summary) {
display: none !important;
}
}
@keithamus It seems that the fix in https://github.com/primer/css/pull/2611 wasn’t sufficient for making the details elements auto-expand for find-in-page matches. See the repro steps above in https://github.com/primer/css/issues/2592#issue-2199903597
So I’ve opened https://github.com/primer/css/pull/2624 with a patch that goes further to completely removed the display: none styling.