Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-expand “details” element for find-in-page and scrolling to fragments #18785

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sideshowbarker
Copy link
Contributor

@sideshowbarker sideshowbarker commented Oct 6, 2023

c8fa1a1

Auto-expand “details” element for find-in-page and scrolling to fragments
https://bugs.webkit.org/show_bug.cgi?id=228843

Reviewed by NOBODY (OOPS!).

This change causes any closed <details> element to be opened
automatically (auto-expanded) if either:

- the user is using find-in-page and a descendant node of the closed
  <details> element contains a match for the given search string
- the user is navigating to a fragment ID and the fragment ID is for a
  descendant element of the closed <details> element

…per the requirements in the HTML spec’s Find-in-page section at
https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found

Otherwise, without this change, when a descendant node of a closed
<details> element contains a match for the given find-in-page search
string, or when navigating to a fragment ID for a descendant element of
a closed <details> element, the closed <details> element isn’t opened
automatically (not auto-expanded).

Further, per the requirements in the HTML spec’s Rendering section at
https://html.spec.whatwg.org/#the-details-and-summary-elements:the-details-element-6,
when the second slot of a <details> element has no “open” attribute,
this change causes that slot to have its “style” attribute set to
“display: block; content-visibility: hidden”.

Otherwise, without this change, when the “open” attribute is removed
from a <details> element, its second slot is removed from the DOM.

This change also introduces the DetailsAutoExpandEnabled preference
(disabled by default), for controlling whether <details> elements get
auto-expanded under the conditions described above.

And this also adds a testRunner.findInPage(target, options) function —
WK2-only — for testing the browser UI find-in-page feature, by calling
into the same code path that gets executed when that feature is used.

* LayoutTests/TestExpectations:
* LayoutTests/accessibility/ios-simulator/aria-details-toggle-summary-expected.txt:
* LayoutTests/accessibility/ios-simulator/destroy-on-press-expected.txt:
* LayoutTests/accessibility/ios-simulator/detail-summary-ios-expected.txt:
* LayoutTests/accessibility/mac/details-summary-expected.txt:
* LayoutTests/accessibility/mac/search-predicate-summary-expected.txt:
* LayoutTests/editing/text-iterator/find-in-page-in-closed-details-expected.txt: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-closed-details.html: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-summary-of-closed-details-expected.txt: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-summary-of-closed-details.html: Added.
* LayoutTests/fast/html/details-children-merge-crash-expected.txt:
* LayoutTests/fast/html/details-element-render-inline-crash-expected.txt:
* LayoutTests/fast/html/details-mouse-click-expected.txt:
* LayoutTests/fast/html/details-summary-tabindex-crash-expected.txt:
* LayoutTests/http/tests/security/cross-origin-blob-transfer-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/inserthorizontalrule-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html-aam/roles-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/interaction/focus/sequential-focus-navigation-and-the-tabindex-attribute/tabindex-getter-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/auto-expand-details-element-fragment-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/closed-details-layout-apis.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-summary-element/activation-behavior-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-summary-element/anchor-with-inline-element-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-details-behavior.tentative-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html-aam/roles-expected.txt:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/tables/table-col-indent-crash-expected.txt:
* LayoutTests/webrtc/legacy-api-expected.txt:
* LayoutTests/webrtc/legacy-api.html:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::expandAllDetailsAncestors):
* Source/WebCore/dom/Node.h:
* Source/WebCore/html/HTMLDetailsElement.cpp:
(WebCore::HTMLDetailsElement::didAddUserAgentShadowRoot):
(WebCore::HTMLDetailsElement::attributeChanged):
* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::scrollToFragmentInternal):
* Source/WebCore/rendering/RenderObject.cpp:
(WebCore::RenderObject::isSkippedContentForLayout const):
* Source/WebKit/WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::findString):
* Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::findInPage):
* Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:
* Tools/WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):

c8fa1a1

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch ✅ 🛠 jsc-armv7
✅ 🛠 watch-sim ✅ 🧪 jsc-armv7-tests

@sideshowbarker sideshowbarker self-assigned this Oct 6, 2023
@sideshowbarker sideshowbarker added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Oct 6, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 3e2f0e6 to d4b5cdf Compare October 6, 2023 22:14
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from d4b5cdf to b381634 Compare October 6, 2023 23:43
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 7, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 8, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from b381634 to a52e8f1 Compare October 8, 2023 15:39
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 8, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 8, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from a52e8f1 to 43342c1 Compare October 8, 2023 18:47
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 8, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 9, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 43342c1 to 42c9300 Compare October 9, 2023 06:29
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 9, 2023
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Oct 25, 2023
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 42c9300 to 0108ecb Compare October 25, 2023 02:56
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Mar 26, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from c7b36b5 to 4175706 Compare March 26, 2024 23:24
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 4175706 to 562ae1a Compare March 27, 2024 00:07
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 562ae1a to 2dbc65c Compare March 27, 2024 00:10
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 2dbc65c to 982c443 Compare March 27, 2024 06:04
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 982c443 to f73ecd1 Compare March 27, 2024 08:25
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from f73ecd1 to 4fe4797 Compare March 27, 2024 18:26
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 4fe4797 to ca94ff0 Compare March 27, 2024 20:18
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from ca94ff0 to 323d4fe Compare April 3, 2024 06:51
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 3, 2024
…ents

https://bugs.webkit.org/show_bug.cgi?id=228843

Reviewed by NOBODY (OOPS!).

This change causes any closed <details> element to be opened
automatically (auto-expanded) if either:

- the user is using find-in-page and a descendant node of the closed
  <details> element contains a match for the given search string
- the user is navigating to a fragment ID and the fragment ID is for a
  descendant element of the closed <details> element

…per the requirements in the HTML spec’s Find-in-page section at
https://html.spec.whatwg.org/#interaction-with-details-and-hidden=until-found

Otherwise, without this change, when a descendant node of a closed
<details> element contains a match for the given find-in-page search
string, or when navigating to a fragment ID for a descendant element of
a closed <details> element, the closed <details> element isn’t opened
automatically (not auto-expanded).

Further, per the requirements in the HTML spec’s Rendering section at
https://html.spec.whatwg.org/#the-details-and-summary-elements:the-details-element-6,
when the second slot of a <details> element has no “open” attribute,
this change causes that slot to have its “style” attribute set to
“display: block; content-visibility: hidden”.

Otherwise, without this change, when the “open” attribute is removed
from a <details> element, its second slot is removed from the DOM.

This change also introduces the DetailsAutoExpandEnabled preference
(disabled by default), for controlling whether <details> elements get
auto-expanded under the conditions described above.

And this also adds a testRunner.findInPage(target, options) function —
WK2-only — for testing the browser UI find-in-page feature, by calling
into the same code path that gets executed when that feature is used.

* LayoutTests/TestExpectations:
* LayoutTests/accessibility/ios-simulator/aria-details-toggle-summary-expected.txt:
* LayoutTests/accessibility/ios-simulator/destroy-on-press-expected.txt:
* LayoutTests/accessibility/ios-simulator/detail-summary-ios-expected.txt:
* LayoutTests/accessibility/mac/details-summary-expected.txt:
* LayoutTests/accessibility/mac/search-predicate-summary-expected.txt:
* LayoutTests/editing/text-iterator/find-in-page-in-closed-details-expected.txt: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-closed-details.html: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-summary-of-closed-details-expected.txt: Added.
* LayoutTests/editing/text-iterator/find-in-page-in-summary-of-closed-details.html: Added.
* LayoutTests/fast/html/details-children-merge-crash-expected.txt:
* LayoutTests/fast/html/details-element-render-inline-crash-expected.txt:
* LayoutTests/fast/html/details-mouse-click-expected.txt:
* LayoutTests/fast/html/details-summary-tabindex-crash-expected.txt:
* LayoutTests/http/tests/security/cross-origin-blob-transfer-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/inserthorizontalrule-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html-aam/roles-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/interaction/focus/sequential-focus-navigation-and-the-tabindex-attribute/tabindex-getter-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/auto-expand-details-element-fragment-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/closed-details-layout-apis.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-summary-element/activation-behavior-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-summary-element/anchor-with-inline-element-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-details-behavior.tentative-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/accname/name/comp_tooltip-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html-aam/roles-expected.txt:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/tables/table-col-indent-crash-expected.txt:
* LayoutTests/webrtc/legacy-api-expected.txt:
* LayoutTests/webrtc/legacy-api.html:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::expandAllDetailsAncestors):
* Source/WebCore/dom/Node.h:
* Source/WebCore/html/HTMLDetailsElement.cpp:
(WebCore::HTMLDetailsElement::didAddUserAgentShadowRoot):
(WebCore::HTMLDetailsElement::attributeChanged):
* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::scrollToFragmentInternal):
* Source/WebCore/rendering/RenderObject.cpp:
(WebCore::RenderObject::isSkippedContentForLayout const):
* Source/WebKit/WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::findString):
* Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::findInPage):
* Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:
* Tools/WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
@sideshowbarker sideshowbarker removed the merging-blocked Applied to prevent a change from being merged label Apr 3, 2024
@sideshowbarker sideshowbarker force-pushed the eng/Implement-auto-expanding-details branch from 323d4fe to c8fa1a1 Compare April 3, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
7 participants