Skip to content

Commit

Permalink
[iOS] Cmd-G after Cmd-E fails to highlight found result
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=245266
rdar://97791726

Reviewed by Wenson Hsieh.

Cmd-E is the keyboard shortcut for "Use Selection For Find". When followed by
Cmd-G, the selection is used to search the text.

In this flow of events, the find panel is not intended to be displayed. Instead,
the found result should simply be flashed (briefly highlighted) and selected.
This functionality is currently unsupported in `WebFoundTextRangeController` as
the highlight (text indicator) is drawn in the find overlay layer, and the layer
is only displayed when a find panel is visible.

To fix, in this scenario, display the text indicator using the UI process.

* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::setTextIndicator const):
* Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.cpp:
(WebKit::WebFoundTextRangeController::decorateTextRangeWithStyle):

Perform the alternate logic if no find overlay layer exits. This approach
matches `UITextView`s logic to differentiate between a full find session (where
and overlay and panel are visible), and the Cmd-E / Cmd-G behavior.

(WebKit::WebFoundTextRangeController::createTextIndicatorForRange):

Factor out text indicator creation into a common method.

(WebKit::WebFoundTextRangeController::setTextIndicatorWithRange):
(WebKit::WebFoundTextRangeController::flashTextIndicatorAndUpdateSelectionWithRange):

Flash (bounce) the text indicator using the UI process via the ChromeClient.

Ensure the selection is updated in the web process.

* Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::setTextIndicator):
* Source/WebKit/WebProcess/WebPage/WebPage.h:

Canonical link: https://commits.webkit.org/254574@main
  • Loading branch information
pxlcoder committed Sep 16, 2022
1 parent 3a46bdd commit 06d20b1
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 4 deletions.
Expand Up @@ -1274,7 +1274,7 @@ void WebChromeClient::storeAppHighlight(WebCore::AppHighlight&& highlight) const

void WebChromeClient::setTextIndicator(const WebCore::TextIndicatorData& indicatorData) const
{
m_page.send(Messages::WebPageProxy::SetTextIndicator(indicatorData, static_cast<uint64_t>(WebCore::TextIndicatorLifetime::Temporary)));
m_page.setTextIndicator(indicatorData);
}

String WebChromeClient::signedPublicKeyAndChallengeString(unsigned keySizeIndex, const String& challengeString, const URL& url) const
Expand Down
25 changes: 22 additions & 3 deletions Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.cpp
Expand Up @@ -33,6 +33,7 @@
#include <WebCore/Editor.h>
#include <WebCore/FocusController.h>
#include <WebCore/Frame.h>
#include <WebCore/FrameSelection.h>
#include <WebCore/FrameView.h>
#include <WebCore/GeometryUtilities.h>
#include <WebCore/GraphicsContext.h>
Expand Down Expand Up @@ -131,7 +132,11 @@ void WebFoundTextRangeController::decorateTextRangeWithStyle(const WebFoundTextR
simpleRange->start.document().markers().addMarker(*simpleRange, WebCore::DocumentMarker::TextMatch);
else if (style == FindDecorationStyle::Highlighted) {
m_highlightedRange = range;
setTextIndicatorWithRange(*simpleRange);

if (m_findPageOverlay)
setTextIndicatorWithRange(*simpleRange);
else
flashTextIndicatorAndUpdateSelectionWithRange(*simpleRange);
}

if (m_findPageOverlay)
Expand Down Expand Up @@ -317,7 +322,7 @@ void WebFoundTextRangeController::drawRect(WebCore::PageOverlay&, WebCore::Graph
}
}

void WebFoundTextRangeController::setTextIndicatorWithRange(const WebCore::SimpleRange& range)
RefPtr<WebCore::TextIndicator> WebFoundTextRangeController::createTextIndicatorForRange(const WebCore::SimpleRange& range, WebCore::TextIndicatorPresentationTransition transition)
{
constexpr int indicatorMargin = 1;

Expand All @@ -332,7 +337,21 @@ void WebFoundTextRangeController::setTextIndicatorWithRange(const WebCore::Simpl
frame->selection().setUpdateAppearanceEnabled(false);
#endif

m_textIndicator = WebCore::TextIndicator::createWithRange(range, options, WebCore::TextIndicatorPresentationTransition::None, WebCore::FloatSize(indicatorMargin, indicatorMargin));
return WebCore::TextIndicator::createWithRange(range, options, transition, WebCore::FloatSize(indicatorMargin, indicatorMargin));
}

void WebFoundTextRangeController::setTextIndicatorWithRange(const WebCore::SimpleRange& range)
{
m_textIndicator = createTextIndicatorForRange(range, WebCore::TextIndicatorPresentationTransition::None);
}

void WebFoundTextRangeController::flashTextIndicatorAndUpdateSelectionWithRange(const WebCore::SimpleRange& range)
{
auto& document = range.startContainer().document();
document.selection().setSelection(WebCore::VisibleSelection(range), WebCore::FrameSelection::defaultSetSelectionOptions(WebCore::UserTriggered));

if (auto textIndicator = createTextIndicatorForRange(range, WebCore::TextIndicatorPresentationTransition::Bounce))
m_webPage->setTextIndicator(textIndicator->data());
}

Vector<WebCore::FloatRect> WebFoundTextRangeController::rectsForTextMatchesInRect(WebCore::IntRect clipRect)
Expand Down
Expand Up @@ -79,7 +79,9 @@ class WebFoundTextRangeController : private WebCore::PageOverlay::Client {
bool mouseEvent(WebCore::PageOverlay&, const WebCore::PlatformMouseEvent&) override;
void drawRect(WebCore::PageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) override;

RefPtr<WebCore::TextIndicator> createTextIndicatorForRange(const WebCore::SimpleRange&, WebCore::TextIndicatorPresentationTransition);
void setTextIndicatorWithRange(const WebCore::SimpleRange&);
void flashTextIndicatorAndUpdateSelectionWithRange(const WebCore::SimpleRange&);

Vector<WebCore::FloatRect> rectsForTextMatchesInRect(WebCore::IntRect clipRect);

Expand Down
5 changes: 5 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Expand Up @@ -4879,6 +4879,11 @@ void WebPage::setActiveOpenPanelResultListener(Ref<WebOpenPanelResultListener>&&
m_activeOpenPanelResultListener = WTFMove(openPanelResultListener);
}

void WebPage::setTextIndicator(const WebCore::TextIndicatorData& indicatorData)
{
send(Messages::WebPageProxy::SetTextIndicator(indicatorData, static_cast<uint64_t>(WebCore::TextIndicatorLifetime::Temporary)));
}

bool WebPage::findStringFromInjectedBundle(const String& target, OptionSet<FindOptions> options)
{
return m_page->findString(target, core(options));
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.h
Expand Up @@ -557,6 +557,8 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
void findStringMatchesFromInjectedBundle(const String&, OptionSet<FindOptions>);
void replaceStringMatchesFromInjectedBundle(const Vector<uint32_t>& matchIndices, const String& replacementText, bool selectionOnly);

void setTextIndicator(const WebCore::TextIndicatorData&);

WebFrame& mainWebFrame() const { return m_mainFrame; }

WebCore::Frame* mainFrame() const; // May return nullptr.
Expand Down

0 comments on commit 06d20b1

Please sign in to comment.