[webkit-reviews] review granted: [Bug 209207] Change TextIterator::rangeLength to not require a live range : [Attachment 394231] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Mar 23 05:27:19 PDT 2020
Antti Koivisto <koivisto at iki.fi> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 209207: Change TextIterator::rangeLength to not require a live range
https://bugs.webkit.org/show_bug.cgi?id=209207
Attachment 394231: Patch
https://bugs.webkit.org/attachment.cgi?id=394231&action=review
--- Comment #11 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 394231
--> https://bugs.webkit.org/attachment.cgi?id=394231
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=394231&action=review
> Source/WebCore/accessibility/AXObjectCache.cpp:1946
> - auto searchRange = Range::create(m_document, startPosition,
endPosition);
> - if (searchRange->collapsed())
> + SimpleRange searchRange { *makeBoundaryPoint(startPosition),
*makeBoundaryPoint(endPosition) };
I sort of like
auto searchRange = SimpleRange { *makeBoundaryPoint(startPosition),
*makeBoundaryPoint(endPosition) };
for these. It is longer but I feel it reads better. It is also allows replacing
constructor call with a function call (or opposite, like here) with minimal
changes.
> Source/WebCore/editing/TextIterator.h:299
> +WEBCORE_EXPORT CharacterCount characterCount(const SimpleRange&, bool
spacesForReplacedElements = false);
The boolean is rather magical in the call sites. Please add enum.
More information about the webkit-reviews
mailing list