Commit aa42c148 authored by Joey Arhar's avatar Joey Arhar Committed by Commit Bot

Highlight active match after scrolling for find-in-page

When using beforematch to expand a content-visibility: hidden-matchable
element asynchronously with an animation, the active match usually
doesn't get highlighted.
This patch makes sure highlighting happens after the scroll, fixing the
issue.

Bug: 1114208
Change-Id: I8dc60aa39461e05d43f94a863514a261e19e4e8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342577Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarvmpstr <vmpstr@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797000}
parent 29189549
...@@ -54,7 +54,7 @@ class CORE_EXPORT DisplayLockDocumentState final ...@@ -54,7 +54,7 @@ class CORE_EXPORT DisplayLockDocumentState final
// Returns true if all activatable locks have been forced. // Returns true if all activatable locks have been forced.
bool ActivatableDisplayLocksForced() const; bool ActivatableDisplayLocksForced() const;
class ScopedForceActivatableDisplayLocks { class CORE_EXPORT ScopedForceActivatableDisplayLocks {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
......
...@@ -442,10 +442,17 @@ void TextFinder::DidFindMatch(int identifier, ...@@ -442,10 +442,17 @@ void TextFinder::DidFindMatch(int identifier,
active_match_index_ + 1, identifier); active_match_index_ + 1, identifier);
} }
} }
OwnerFrame().GetFrame()->GetDocument()->Markers().AddTextMatchMarker( DocumentMarkerController& marker_controller =
EphemeralRange(result_range), OwnerFrame().GetFrame()->GetDocument()->Markers();
found_active_match ? TextMatchMarker::MatchStatus::kActive EphemeralRange ephemeral_result_range(result_range);
: TextMatchMarker::MatchStatus::kInactive); // Scroll() may have added a match marker to this range already.
if (!marker_controller.FirstMarkerIntersectingEphemeralRange(
ephemeral_result_range, DocumentMarker::MarkerTypes::TextMatch())) {
marker_controller.AddTextMatchMarker(
EphemeralRange(result_range),
found_active_match ? TextMatchMarker::MatchStatus::kActive
: TextMatchMarker::MatchStatus::kInactive);
}
find_matches_cache_.push_back(FindMatch(result_range, current_total_matches)); find_matches_cache_.push_back(FindMatch(result_range, current_total_matches));
} }
...@@ -825,6 +832,21 @@ void TextFinder::Scroll(std::unique_ptr<AsyncScrollContext> context) { ...@@ -825,6 +832,21 @@ void TextFinder::Scroll(std::unique_ptr<AsyncScrollContext> context) {
OwnerFrame().GetFrameView()->ConvertToRootFrame( OwnerFrame().GetFrameView()->ConvertToRootFrame(
ComputeTextRect(EphemeralRange(context->range)))); ComputeTextRect(EphemeralRange(context->range))));
} }
// DidFindMatch will race against this to add a text match marker to this
// range. In the case where the match is hidden and the beforematch event (or
// anything else) reveals the range in between DidFindMatch and this function,
// we need to add the marker again or else it won't show up at all.
EphemeralRange ephemeral_range(context->range);
DocumentMarkerController& marker_controller =
OwnerFrame().GetFrame()->GetDocument()->Markers();
if (!context->options.run_synchronously_for_testing &&
!marker_controller.FirstMarkerIntersectingEphemeralRange(
ephemeral_range, DocumentMarker::MarkerTypes::TextMatch())) {
marker_controller.AddTextMatchMarker(ephemeral_range,
TextMatchMarker::MatchStatus::kActive);
SetMarkerActive(context->range, true);
}
} }
} // namespace blink } // namespace blink
...@@ -11,12 +11,15 @@ ...@@ -11,12 +11,15 @@
#include "third_party/blink/public/web/web_document.h" #include "third_party/blink/public/web/web_document.h"
#include "third_party/blink/renderer/bindings/core/v8/script_source_code.h" #include "third_party/blink/renderer/bindings/core/v8/script_source_code.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h" #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
#include "third_party/blink/renderer/core/display_lock/display_lock_document_state.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/node_list.h" #include "third_party/blink/renderer/core/dom/node_list.h"
#include "third_party/blink/renderer/core/dom/range.h" #include "third_party/blink/renderer/core/dom/range.h"
#include "third_party/blink/renderer/core/dom/shadow_root.h" #include "third_party/blink/renderer/core/dom/shadow_root.h"
#include "third_party/blink/renderer/core/editing/ephemeral_range.h" #include "third_party/blink/renderer/core/editing/ephemeral_range.h"
#include "third_party/blink/renderer/core/editing/finder/find_in_page_coordinates.h" #include "third_party/blink/renderer/core/editing/finder/find_in_page_coordinates.h"
#include "third_party/blink/renderer/core/editing/markers/document_marker.h"
#include "third_party/blink/renderer/core/editing/markers/document_marker_controller.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h" #include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h" #include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/visual_viewport.h" #include "third_party/blink/renderer/core/frame/visual_viewport.h"
...@@ -25,6 +28,8 @@ ...@@ -25,6 +28,8 @@
#include "third_party/blink/renderer/core/layout/text_autosizer.h" #include "third_party/blink/renderer/core/layout/text_autosizer.h"
#include "third_party/blink/renderer/core/page/page.h" #include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/script/classic_script.h" #include "third_party/blink/renderer/core/script/classic_script.h"
#include "third_party/blink/renderer/core/testing/sim/sim_request.h"
#include "third_party/blink/renderer/core/testing/sim/sim_test.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h" #include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support.h" #include "third_party/blink/renderer/platform/testing/testing_platform_support.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h" #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
...@@ -61,6 +66,14 @@ class TextFinderTest : public testing::Test { ...@@ -61,6 +66,14 @@ class TextFinderTest : public testing::Test {
Persistent<TextFinder> text_finder_; Persistent<TextFinder> text_finder_;
}; };
class TextFinderSimTest : public SimTest {
public:
TextFinder& GetTextFinder() {
return WebLocalFrameImpl::FromFrame(GetDocument().GetFrame())
->EnsureTextFinder();
}
};
v8::Local<v8::Value> TextFinderTest::EvalJs(const std::string& script) { v8::Local<v8::Value> TextFinderTest::EvalJs(const std::string& script) {
return ClassicScript::CreateUnspecifiedScript( return ClassicScript::CreateUnspecifiedScript(
ScriptSourceCode(script.c_str())) ScriptSourceCode(script.c_str()))
...@@ -715,6 +728,47 @@ TEST_F(TextFinderTest, BeforeMatchEventRemoveElement) { ...@@ -715,6 +728,47 @@ TEST_F(TextFinderTest, BeforeMatchEventRemoveElement) {
// TODO(jarhar): Write more tests here once we decide on a behavior here: // TODO(jarhar): Write more tests here once we decide on a behavior here:
// https://github.com/WICG/display-locking/issues/150 // https://github.com/WICG/display-locking/issues/150
TEST_F(TextFinderSimTest, BeforeMatchEventAsyncExpandHighlight) {
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
.hidden {
content-visibility: hidden-matchable;
}
</style>
<div id=hiddenid class=hidden>hidden</div>
<script>
hiddenid.addEventListener('beforematch', () => {
setTimeout(() => {
hiddenid.classList.remove('hidden');
}, 0);
});
</script>
)HTML");
Compositor().BeginFrame();
auto forced_activatable_locks = GetDocument()
.GetDisplayLockDocumentState()
.GetScopedForceActivatableLocks();
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kFindInPage);
GetTextFinder().Find(/*identifier=*/0, WebString(String("hidden")),
*mojom::blink::FindOptions::New(),
/*wrap_within_frame=*/false);
Compositor().BeginFrame();
Compositor().BeginFrame();
HeapVector<Member<DocumentMarker>> markers =
GetDocument().Markers().Markers();
ASSERT_EQ(markers.size(), 1u);
DocumentMarker* marker = markers[0];
EXPECT_TRUE(marker->GetType() == DocumentMarker::kTextMatch);
}
TEST_F(TextFinderTest, FindTextAcrossCommentNode) { TEST_F(TextFinderTest, FindTextAcrossCommentNode) {
GetDocument().body()->setInnerHTML( GetDocument().body()->setInnerHTML(
"<span>abc</span><!--comment--><span>def</span>"); "<span>abc</span><!--comment--><span>def</span>");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment