Skip to content

Commit

Permalink
[Cocoa] Find-in-page should match text inside image overlays
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=226704

Reviewed by Tim Horton.

Source/WebCore:

Add a new TextIteratorBehavior to allow TextIterator to descend into image overlay content, and use this option
when creating TextIterators for find-in-page. See WebKit/ChangeLog for more details.

Test: WebKit.FindTextInImageOverlay

* editing/TextIterator.cpp:
(WebCore::TextIterator::handleReplacedElement):
(WebCore::findIteratorOptions):
* editing/TextIteratorBehavior.h:

Source/WebKit:

Add the `PaintAllContent` and `PaintBackgrounds` text indicator options when generating a TextIndicator for
selected content inside an image overlay. See WebCore/ChangeLog for more details.

* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindIndicator):
* WebProcess/WebPage/ios/FindControllerIOS.mm:
(WebKit::findTextIndicatorOptions):
(WebKit::FindIndicatorOverlayClientIOS::drawRect):
(WebKit::FindController::updateFindIndicator):

Tools:

Add an API test to verify that text inside image overlays is visible to find-in-page.

* TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:


Canonical link: https://commits.webkit.org/238558@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278560 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
whsieh committed Jun 7, 2021
1 parent 0940dae commit e84725b
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 13 deletions.
17 changes: 17 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
2021-06-07 Wenson Hsieh <wenson_hsieh@apple.com>

[Cocoa] Find-in-page should match text inside image overlays
https://bugs.webkit.org/show_bug.cgi?id=226704

Reviewed by Tim Horton.

Add a new TextIteratorBehavior to allow TextIterator to descend into image overlay content, and use this option
when creating TextIterators for find-in-page. See WebKit/ChangeLog for more details.

Test: WebKit.FindTextInImageOverlay

* editing/TextIterator.cpp:
(WebCore::TextIterator::handleReplacedElement):
(WebCore::findIteratorOptions):
* editing/TextIteratorBehavior.h:

2021-06-07 Alicia Boya García <aboya@igalia.com>

[GStreamer] Add clang TSA annotations: MainThreadNotifier
Expand Down
14 changes: 12 additions & 2 deletions Source/WebCore/editing/TextIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,16 @@ bool TextIterator::handleReplacedElement()
}
}

if (m_behaviors.contains(TextIteratorBehavior::EntersImageOverlays) && is<HTMLElement>(m_node) && downcast<HTMLElement>(*m_node).hasImageOverlay()) {
if (auto shadowRoot = makeRefPtr(m_node->shadowRoot())) {
m_node = shadowRoot.get();
pushFullyClippedState(m_fullyClippedStack, *m_node);
m_offset = 0;
return false;
}
ASSERT_NOT_REACHED();
}

m_hasEmitted = true;

if (m_behaviors.contains(TextIteratorBehavior::EmitsObjectReplacementCharacters) && renderer.isReplaced()) {
Expand Down Expand Up @@ -2454,9 +2464,9 @@ String plainTextReplacingNoBreakSpace(const SimpleRange& range, TextIteratorBeha
return plainText(range, defaultBehaviors, isDisplayString).replace(noBreakSpace, ' ');
}

static TextIteratorBehaviors findIteratorOptions(FindOptions options)
static constexpr TextIteratorBehaviors findIteratorOptions(FindOptions options)
{
TextIteratorBehaviors iteratorOptions { TextIteratorBehavior::EntersTextControls, TextIteratorBehavior::ClipsToFrameAncestors };
TextIteratorBehaviors iteratorOptions { TextIteratorBehavior::EntersTextControls, TextIteratorBehavior::ClipsToFrameAncestors, TextIteratorBehavior::EntersImageOverlays };
if (!options.contains(DoNotTraverseFlatTree))
iteratorOptions.add(TextIteratorBehavior::TraversesFlatTree);
return iteratorOptions;
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/editing/TextIteratorBehavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ enum class TextIteratorBehavior : uint16_t {
ClipsToFrameAncestors = 1 << 8,

TraversesFlatTree = 1 << 9,

EntersImageOverlays = 1 << 10,
};

using TextIteratorBehaviors = OptionSet<TextIteratorBehavior>;
Expand Down
17 changes: 17 additions & 0 deletions Source/WebKit/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
2021-06-07 Wenson Hsieh <wenson_hsieh@apple.com>

[Cocoa] Find-in-page should match text inside image overlays
https://bugs.webkit.org/show_bug.cgi?id=226704

Reviewed by Tim Horton.

Add the `PaintAllContent` and `PaintBackgrounds` text indicator options when generating a TextIndicator for
selected content inside an image overlay. See WebCore/ChangeLog for more details.

* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindIndicator):
* WebProcess/WebPage/ios/FindControllerIOS.mm:
(WebKit::findTextIndicatorOptions):
(WebKit::FindIndicatorOverlayClientIOS::drawRect):
(WebKit::FindController::updateFindIndicator):

2021-06-07 Aditya Keerthi <akeerthi@apple.com>

[iOS] Unexpected scrolling when switching focus from a text input to a select element
Expand Down
6 changes: 5 additions & 1 deletion Source/WebKit/WebProcess/WebPage/FindController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,11 @@ void FindController::hideFindUI()

bool FindController::updateFindIndicator(Frame& selectedFrame, bool isShowingOverlay, bool shouldAnimate)
{
auto indicator = TextIndicator::createWithSelectionInFrame(selectedFrame, { TextIndicatorOption::IncludeMarginIfRangeMatchesSelection }, shouldAnimate ? TextIndicatorPresentationTransition::Bounce : TextIndicatorPresentationTransition::None);
OptionSet<TextIndicatorOption> textIndicatorOptions { TextIndicatorOption::IncludeMarginIfRangeMatchesSelection };
if (auto selectedRange = selectedFrame.selection().selection().range(); selectedRange && HTMLElement::isInsideImageOverlay(*selectedRange))
textIndicatorOptions.add({ TextIndicatorOption::PaintAllContent, TextIndicatorOption::PaintBackgrounds });

auto indicator = TextIndicator::createWithSelectionInFrame(selectedFrame, textIndicatorOptions, shouldAnimate ? TextIndicatorPresentationTransition::Bounce : TextIndicatorPresentationTransition::None);
if (!indicator)
return false;

Expand Down
12 changes: 9 additions & 3 deletions Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@
const int totalHorizontalMargin = 1;
const int totalVerticalMargin = 1;

constexpr OptionSet<TextIndicatorOption> findTextIndicatorOptions { TextIndicatorOption::IncludeMarginIfRangeMatchesSelection, TextIndicatorOption::DoNotClipToVisibleRect };
static OptionSet<TextIndicatorOption> findTextIndicatorOptions(const Frame& frame)
{
OptionSet<TextIndicatorOption> options { TextIndicatorOption::IncludeMarginIfRangeMatchesSelection, TextIndicatorOption::DoNotClipToVisibleRect };
if (auto selectedRange = frame.selection().selection().range(); selectedRange && HTMLElement::isInsideImageOverlay(*selectedRange))
options.add({ TextIndicatorOption::PaintAllContent, TextIndicatorOption::PaintBackgrounds });
return options;
};

static constexpr auto highlightColor = SRGBA<uint8_t> { 255, 228, 56 };

Expand All @@ -64,7 +70,7 @@

// If the page scale changed, we need to paint a new TextIndicator.
if (m_textIndicator->contentImageScaleFactor() != scaleFactor)
m_textIndicator = TextIndicator::createWithSelectionInFrame(m_frame, findTextIndicatorOptions, TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));
m_textIndicator = TextIndicator::createWithSelectionInFrame(m_frame, findTextIndicatorOptions(m_frame), TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));

if (!m_textIndicator)
return;
Expand All @@ -91,7 +97,7 @@
m_isShowingFindIndicator = false;
}

auto textIndicator = TextIndicator::createWithSelectionInFrame(selectedFrame, findTextIndicatorOptions, TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));
auto textIndicator = TextIndicator::createWithSelectionInFrame(selectedFrame, findTextIndicatorOptions(selectedFrame), TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));
if (!textIndicator)
return false;

Expand Down
11 changes: 11 additions & 0 deletions Tools/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
2021-06-07 Wenson Hsieh <wenson_hsieh@apple.com>

[Cocoa] Find-in-page should match text inside image overlays
https://bugs.webkit.org/show_bug.cgi?id=226704

Reviewed by Tim Horton.

Add an API test to verify that text inside image overlays is visible to find-in-page.

* TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:

2021-06-07 Carlos Garcia Campos <cgarcia@igalia.com>

[GTK][WPE] Signal "window-object-cleared" not emitted unless frame js context is get before
Expand Down
39 changes: 32 additions & 7 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#import "PlatformUtilities.h"
#import "TestNavigationDelegate.h"
#import "TestWKWebView.h"
#import "WKWebViewConfigurationExtras.h"
#import <WebKit/WKWebViewPrivate.h>
#import <wtf/RetainPtr.h>

Expand All @@ -38,10 +39,10 @@
NSTextFinderAsynchronousDocumentFindOptionsWrap = 1 << 1,
} NSTextFinderAsynchronousDocumentFindOptions;

NSTextFinderAsynchronousDocumentFindOptions noFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)0;
NSTextFinderAsynchronousDocumentFindOptions backwardsFindOptions =NSTextFinderAsynchronousDocumentFindOptionsBackwards;
NSTextFinderAsynchronousDocumentFindOptions wrapFindOptions =NSTextFinderAsynchronousDocumentFindOptionsWrap;
NSTextFinderAsynchronousDocumentFindOptions wrapBackwardsFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)(NSTextFinderAsynchronousDocumentFindOptionsWrap | NSTextFinderAsynchronousDocumentFindOptionsBackwards);
constexpr auto noFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)0;
constexpr auto backwardsFindOptions = NSTextFinderAsynchronousDocumentFindOptionsBackwards;
constexpr auto wrapFindOptions = NSTextFinderAsynchronousDocumentFindOptionsWrap;
constexpr auto wrapBackwardsFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)(NSTextFinderAsynchronousDocumentFindOptionsWrap | NSTextFinderAsynchronousDocumentFindOptionsBackwards);

@protocol NSTextFinderAsynchronousDocumentFindMatch <NSObject>
@property (retain, nonatomic, readonly) NSArray *textRects;
Expand All @@ -57,10 +58,10 @@ - (void)replaceMatches:(NSArray<FindMatch> *)matches withString:(NSString *)repl

@end

typedef struct {
struct FindResult {
RetainPtr<NSArray> matches;
BOOL didWrap;
} FindResult;
BOOL didWrap { NO };
};

static FindResult findMatches(WKWebView *webView, NSString *findString, NSTextFinderAsynchronousDocumentFindOptions findOptions = noFindOptions, NSUInteger maxResults = NSUIntegerMax)
{
Expand Down Expand Up @@ -295,4 +296,28 @@ static NSUInteger replaceMatches(WKWebView *webView, NSArray<FindMatch> *matches
EXPECT_WK_STREQ("hi hi", [webView stringByEvaluatingJavaScript:@"document.body.textContent"]);
}

#if ENABLE(IMAGE_EXTRACTION)

TEST(WebKit, FindTextInImageOverlay)
{
auto configuration = retainPtr([WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES]);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400) configuration:configuration.get()]);
[webView synchronouslyLoadTestPageNamed:@"simple-image-overlay"];
{
auto [matches, didWrap] = findMatches(webView.get(), @"foobar");
EXPECT_EQ(1U, [matches count]);
EXPECT_FALSE(didWrap);
}

[webView evaluateJavaScript:@"document.body.appendChild(document.createTextNode('foobar'))" completionHandler:nil];

{
auto [matches, didWrap] = findMatches(webView.get(), @"foobar");
EXPECT_EQ(2U, [matches count]);
EXPECT_FALSE(didWrap);
}
}

#endif // ENABLE(IMAGE_EXTRACTION)

#endif // !PLATFORM(IOS_FAMILY)

0 comments on commit e84725b

Please sign in to comment.