Implement hidden=until-found for find-in-page and ElementFragments [chromium/src : main]
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
Joey Arhar uploaded patch set #7 to this change.
Implement hidden=until-found for find-in-page and ElementFragments
This patch implements the hidden=until-found attribute, which is a
replacement for the content-visibility:hidden-matchable CSS property
with two key differences:
1. It's an HTML attribute instead of a CSS property.
2. The attribute is automatically removed by the browser when the
content is revealed, instead of requiring the page to remove it.
Bug: 1055002
Change-Id: I53cf79cf10b778755f1798ae68443c7eb2c572b6
---
M third_party/blink/renderer/core/display_lock/display_lock_context.cc
M third_party/blink/renderer/core/display_lock/display_lock_context.h
M third_party/blink/renderer/core/dom/element.cc
M third_party/blink/renderer/core/editing/finder/text_finder.cc
M third_party/blink/renderer/core/html/html_details_element.cc
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/html/html_element.h
M third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.cc
A third_party/blink/web_tests/editing/text-iterator/auto-expand-until-found.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-beforematch.html
A third_party/blink/web_tests/external/wpt/html/dom/elements/global-attributes/hidden-until-found-fragment-navigation-nested.tentative.html
A third_party/blink/web_tests/external/wpt/html/dom/elements/global-attributes/hidden-until-found-fragment-navigation.tentative.html
12 files changed, 200 insertions(+), 17 deletions(-)
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Attention is currently required from: vmpstr.
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/30013.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
vmpstr (Gerrit)
Attention is currently required from: Joey Arhar.
3 comments:
File third_party/blink/renderer/core/display_lock/display_lock_context.h:
Patch Set #8, Line 428:
bool for_details_element_ = false;This is presumably now unused?
File third_party/blink/renderer/core/editing/finder/text_finder.cc:
Patch Set #8, Line 104:
// We need to update the style and layout since the event dispatched mayJust in terms of timing, does this thing have the event? Or does the event come later?
I think that we need to reveal hidden ancestors before we fire the event, but the comments here seem to suggest that we first fire an event then reveal the contents
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(Can a match be found in an iframe? I'm not sure this traversal crosses boundaries or whether that's possible to have.
Also, I don't think it's possible for us to call this on a node in a visibility:hidden or display:none subtree, but can you verify?
I'm wondering if just removing until-found without any extra checks is the right thing or whether it might be a little too aggressive.
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
3 comments:
File third_party/blink/renderer/core/display_lock/display_lock_context.h:
Patch Set #8, Line 428:
bool for_details_element_ = false;This is presumably now unused?
Done
File third_party/blink/renderer/core/editing/finder/text_finder.cc:
Patch Set #8, Line 104:
// We need to update the style and layout since the event dispatched mayJust in terms of timing, does this thing have the event? Or does the event come later? […]
I agree, the event should be fired after the attribute is removed.
I have a comment in HTMLElement::RevealHiddenUntilFoundAncestors to do so, I plan to fix the timing based on our conversation yesterday and move the firing of the beforematch event there in a later patch.
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
Can a match be found in an iframe? I'm not sure this traversal crosses boundaries or whether that's […]
So I just spend a bunch of time writing a test for this and debugging why it didnt end up working...
It appears that each LocalFrame has its own TextFinder, and I think that the browser process is responsible for deciding which one to search in...
For some reason, the async requestAnimationFrame task to reveal the hidden=until-found matches inside an iframe never fires...
I added tests for this, but I might need to look at this some more in a later patch or something.(I also tried making the traversal cross boundaries but that didn't actually make it work)
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
vmpstr (Gerrit)
Attention is currently required from: Joey Arhar.
1 comment:
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
So I just spend a bunch of time writing a test for this and debugging why it didnt end up working... […]
Interesting. We should dig into this to understand exactly how it works. We definitely need to search in a separate process for something like an OOPIF, so I wonder how we can make it work if we have something like
<div hidden=until-found>
<iframe src="example.com/match-is-here.html></iframe>
</div>
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
1 comment:
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
Interesting. We should dig into this to understand exactly how it works. […]
I'm really at a loss for understanding why Document::EnqueueAnimationFrameTask isn't firing its callback in this case... do you have any ideas? Or know anyone I could ask?
I added tests for hidden=until-found around an iframe and within an iframe. Do you think it would be worth landing this patch now and fixing those tests later?
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
vmpstr (Gerrit)
Attention is currently required from: Joey Arhar.
1 comment:
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
I'm really at a loss for understanding why Document::EnqueueAnimationFrameTask isn't firing its call […]
The problem I think is that we are probably throttling the frame because of the hidden ancestor. We do need to figure out how to resolve this problem though. Something like this: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=4623;drc=23c968012801252f08157091046e609ef8e5a3ed;bpv=1;bpt=1
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
1 comment:
Patchset:
note to self: i think i still need to add a flag
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
2 comments:
Patchset:
note to self: i think i still need to add a flag
Nevermind, its already behind the beforematch flag.
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
The problem I think is that we are probably throttling the frame because of the hidden ancestor. […]
I want to address this for both auto-expanding details and hidden=until-found in a separate patch. Is it ok if we land this as is? I filed a bug here: https://bugs.chromium.org/p/chromium/issues/detail?id=1250847
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
1 comment:
File third_party/blink/renderer/core/editing/finder/text_finder.cc:
Patch Set #8, Line 104:
// We need to update the style and layout since the event dispatched may
I agree, the event should be fired after the attribute is removed. […]
I filed a bug to do this here: https://bugs.chromium.org/p/chromium/issues/detail?id=1250851
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
vmpstr (Gerrit)
Attention is currently required from: Joey Arhar.
2 comments:
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
I want to address this for both auto-expanding details and hidden=until-found in a separate patch. […]
Yep that's fine, reference the bug here? with a comment explaining
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #9, Line 2060:
bool HTMLElement::RevealHiddenUntilFoundAncestors(const Node& node) {This function is probably better suited for DisplayLockUtilities. WDYT?
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Attention is currently required from: vmpstr.
2 comments:
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #8, Line 2049:
if (EqualIgnoringASCIICase(
Yep that's fine, reference the bug here? with a comment explaining
I added a TODO in TextFinder because the issue is more relevant to the code there than it is to the code here.
File third_party/blink/renderer/core/html/html_element.cc:
Patch Set #9, Line 2060:
bool HTMLElement::RevealHiddenUntilFoundAncestors(const Node& node) {
This function is probably better suited for DisplayLockUtilities. […]
Done
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
vmpstr (Gerrit)
Attention is currently required from: Joey Arhar.
Patch set 10:Code-Review +1
1 comment:
File third_party/blink/renderer/core/editing/finder/text_finder.cc:
Patch Set #8, Line 104:
// We need to update the style and layout since the event dispatched may
I filed a bug to do this here: https://bugs.chromium. […]
Ack
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Patch set 10:Commit-Queue +2
Joey Arhar (Gerrit)
Joey Arhar uploaded patch set #11 to this change.
Implement hidden=until-found for find-in-page and ElementFragments
This patch implements the hidden=until-found attribute, which is a
replacement for the content-visibility:hidden-matchable CSS property
with two key differences:
1. It's an HTML attribute instead of a CSS property.
2. The attribute is automatically removed by the browser when the
content is revealed, instead of requiring the page to remove it.
Bug: 1055002
Change-Id: I53cf79cf10b778755f1798ae68443c7eb2c572b6
---
M third_party/blink/renderer/core/display_lock/display_lock_context.cc
M third_party/blink/renderer/core/display_lock/display_lock_context.h
M third_party/blink/renderer/core/display_lock/display_lock_utilities.cc
M third_party/blink/renderer/core/display_lock/display_lock_utilities.h
M third_party/blink/renderer/core/dom/element.cc
M third_party/blink/renderer/core/editing/finder/text_finder.cc
M third_party/blink/renderer/core/html/html_details_element.cc
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.cc
A third_party/blink/web_tests/editing/text-iterator/auto-expand-until-found.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-around-iframe-expected.txt
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-around-iframe.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-beforematch.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-within-iframe-expected.txt
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-within-iframe.html
A third_party/blink/web_tests/editing/text-iterator/resources/hidden-until-found-around-iframe-source.html
A third_party/blink/web_tests/editing/text-iterator/resources/hidden-until-found-within-iframe-source.html
A third_party/blink/web_tests/wpt_internal/display-lock/beforematch/hidden-until-found-fragment-navigation-nested.html
A third_party/blink/web_tests/wpt_internal/display-lock/beforematch/hidden-until-found-fragment-navigation.html
19 files changed, 286 insertions(+), 20 deletions(-)
To view, visit change 2858763. To unsubscribe, or for help writing mail filters, visit settings.
Joey Arhar (Gerrit)
Patch set 11:Commit-Queue +2
Joey Arhar (Gerrit)
Patch set 12:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Implement hidden=until-found for find-in-page and ElementFragments
This patch implements the hidden=until-found attribute, which is a
replacement for the content-visibility:hidden-matchable CSS property
with two key differences:
1. It's an HTML attribute instead of a CSS property.
2. The attribute is automatically removed by the browser when the
content is revealed, instead of requiring the page to remove it.
Bug: 1055002
Change-Id: I53cf79cf10b778755f1798ae68443c7eb2c572b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2858763
Commit-Queue: Joey Arhar <jar...@chromium.org>
Reviewed-by: vmpstr <vmp...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#924142}
---
M third_party/blink/renderer/core/display_lock/display_lock_context.cc
M third_party/blink/renderer/core/display_lock/display_lock_context.h
M third_party/blink/renderer/core/display_lock/display_lock_utilities.cc
M third_party/blink/renderer/core/display_lock/display_lock_utilities.h
M third_party/blink/renderer/core/dom/element.cc
M third_party/blink/renderer/core/editing/finder/text_finder.cc
M third_party/blink/renderer/core/html/html_details_element.cc
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/page/scrolling/element_fragment_anchor.cc
A third_party/blink/web_tests/editing/text-iterator/auto-expand-until-found.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-around-iframe-expected.txt
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-around-iframe.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-beforematch.html
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-within-iframe-expected.txt
A third_party/blink/web_tests/editing/text-iterator/hidden-until-found-within-iframe.html
A third_party/blink/web_tests/editing/text-iterator/resources/hidden-until-found-around-iframe-source.html
A third_party/blink/web_tests/editing/text-iterator/resources/hidden-until-found-within-iframe-source.html
A third_party/blink/web_tests/wpt_internal/display-lock/beforematch/hidden-until-found-fragment-navigation-nested.html
A third_party/blink/web_tests/wpt_internal/display-lock/beforematch/hidden-until-found-fragment-navigation.html
19 files changed, 286 insertions(+), 20 deletions(-)