[webkit-reviews] review granted: [Bug 209207] Change TextIterator::rangeLength to not require a live range : [Attachment 394231] Patch

[bugzilla-daemon at webkit.org]

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.