Move unrelated code out of TextIterator#60819
Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom Mar 18, 2026
Merged
Move unrelated code out of TextIterator#60819webkit-commit-queue merged 1 commit intoWebKit:mainfrom
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Conversation
colelao
added
the
HTML Editing
Collaborator
|
EWS run on previous version of this PR (hash 8cebce5) Details
|
🛠 vision-apple
brentfulgham
approved these changes
Mar 18, 2026
Contributor
brentfulgham
left a comment
brentfulgham
left a comment
There was a problem hiding this comment.
This clean-up looks nice. Please consider renaming the files (or the classname), since it's weird to have a class defined in a file with a totally different name.
r=me
| @@ -138,6 +134,7 @@ class SearchBuffer { | |||
| const bool m_targetRequiresKanaWorkaround; | |||
| Vector<char16_t> m_normalizedTarget; | |||
| mutable Vector<char16_t> m_normalizedMatch; | |||
| ICUSearcher m_ICUSearcher; | |||
Contributor
There was a problem hiding this comment.
Do we always need an ICUSearcher object? Should it be lazily initialized if someone doesn't search?
Contributor
Author
There was a problem hiding this comment.
Yes, the SearchBuffer directly relies on the ICUSearcher, and it is initialized in the SearchBuffer constructor.
| || (m_options.contains(FindOption::AtWordEnds) && !isWordEndMatch(matchStart, matchedLength))) { | ||
| if ((m_targetRequiresKanaWorkaround && isBadMatch(m_buffer.subspan(matchStart, matchedLength), m_normalizedTarget.span(), m_normalizedMatch)) | ||
| || (m_options.contains(FindOption::AtWordStarts) && !isWordStartMatch(m_buffer, matchStart, matchedLength, m_options)) | ||
| || (m_options.contains(FindOption::AtWordEnds) && !isWordEndMatch(m_buffer, matchStart, matchedLength, m_options))) { |
Contributor
There was a problem hiding this comment.
Are all of these changes needed for the switch to an external TextSearchICU object? It seems like some of this is new work related to your search improvements. I think we should keep that separate from this task.
Contributor
Author
There was a problem hiding this comment.
I think isBadMatch, isWordStartMatch, and isWordEndMatch are placed more appropriately in ICUSearcher, so these changes are needed.
rniwa
reviewed
Mar 18, 2026
rniwa
reviewed
Mar 18, 2026
| lockSearcher(); | ||
|
|
||
| SUPPRESS_FORWARD_DECL_ARG UStringSearch* searcher = WebCore::searcher(); | ||
| SUPPRESS_FORWARD_DECL_ARG UStringSearch* searcher = m_ICUSearcher.searcher(); |
Member
There was a problem hiding this comment.
We should probably encapsulate all uses of UStringSearch* into SearchICU interface in a follow up change.
rniwa
approved these changes
Mar 18, 2026
colelao
force-pushed
the
eng/Move-unrelated-code-out-of-TextIterator
branch
from
8cebce5 to
c1dfe00
Compare
Collaborator
|
EWS run on previous version of this PR (hash c1dfe00) Details
|
webkit-ews-buildbot
added
the
merging-blocked
colelao
removed
the
merging-blocked
colelao
force-pushed
the
eng/Move-unrelated-code-out-of-TextIterator
branch
from
c1dfe00 to
e8045c0
Compare
Collaborator
|
EWS run on current version of this PR (hash e8045c0) Details |
colelao
added
the
safe-merge-queue
webkit-ews-buildbot
added
the
merging-blocked
colelao
removed
the
merging-blocked
webkit-ews-buildbot
added
merge-queue
Collaborator
|
Safe-Merge-Queue: Build #87024. |
https://bugs.webkit.org/show_bug.cgi?id=310147 rdar://172786702 Reviewed by Brent Fulgham and Ryosuke Niwa. There are a lot of helper functions in TextIterator.cpp that could be moved into it's own file. This change also allows other clients to use some of these helper functions. * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/dom/FragmentDirectiveRangeFinder.cpp: * Source/WebCore/editing/TextIterator.cpp: (WebCore::SearchBuffer::SearchBuffer): (WebCore::SearchBuffer::reachedBreak): (WebCore::SearchBuffer::search): (WebCore::foldQuoteMarkAndReplaceNoBreakSpace): Deleted. (WebCore::foldQuoteMarks): Deleted. (WebCore::createSearcher): Deleted. (WebCore::searcher): Deleted. (WebCore::lockSearcher): Deleted. (WebCore::unlockSearcher): Deleted. (WebCore::isKanaLetter): Deleted. (WebCore::isSmallKanaLetter): Deleted. (WebCore::composedVoicedSoundMark): Deleted. (WebCore::isCombiningVoicedSoundMark): Deleted. (WebCore::containsKanaLetters): Deleted. (WebCore::normalizeCharacters): Deleted. (WebCore::isNonLatin1Separator): Deleted. (WebCore::isSeparator): Deleted. (WebCore::SearchBuffer::~SearchBuffer): Deleted. (WebCore::SearchBuffer::isBadMatch const): Deleted. (WebCore::SearchBuffer::isWordEndMatch const): Deleted. (WebCore::SearchBuffer::isWordStartMatch const): Deleted. * Source/WebCore/editing/TextIterator.h: * Source/WebCore/editing/TextSearchICU.cpp: Added. (WebCore::createSearcher): (WebCore::globalSearcher): (WebCore::ICUSearcher::ICUSearcher): (WebCore::ICUSearcher::~ICUSearcher): (WebCore::ICUSearcher::lock): (WebCore::ICUSearcher::unlock): (WebCore::ICUSearcher::reset): (WebCore::ICUSearcher::searcher): (WebCore::isBadMatch): (WebCore::isWordStartMatch): (WebCore::isWordEndMatch): (WebCore::normalizeCharacters): (WebCore::foldQuoteMarkAndReplaceNoBreakSpace): (WebCore::isSeparator): (WebCore::containsKanaLetters): (WebCore::foldQuoteMarks): (WebCore::isKanaLetter): (WebCore::isSmallKanaLetter): (WebCore::composedVoicedSoundMark): (WebCore::isCombiningVoicedSoundMark): (WebCore::isNonLatin1Separator): * Source/WebCore/editing/TextSearchICU.h: Added. * Source/WebCore/page/text-extraction/TextExtraction.cpp: * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: * Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm: Canonical link: https://commits.webkit.org/309514@main
webkit-commit-queue
force-pushed
the
eng/Move-unrelated-code-out-of-TextIterator
branch
from
e8045c0 to
5f9b798
Compare
Collaborator
|
Committed 309514@main (5f9b798): https://commits.webkit.org/309514@main Reviewed commits have been landed. Closing PR #60819 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
5f9b798
e8045c0