Bug 208906 – Change all return values in TextIterator header from live ranges to SimpleRange

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Darin Adler]

Change all return values in TextIterator header from live ranges to SimpleRange

[Darin Adler]

Created attachment 393207 [details]
Patch

[Darin Adler]

Hooray this one passed all the tests on the first try

[Antti Koivisto]

Comment on attachment 393207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393207&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:2667
> +    Node& node = it.atEnd() ? searchRange->startContainer() : it.range().start.container.get();

auto

> Source/WebCore/editing/TextIterator.cpp:1494
> -    Ref<Range> r = m_underlyingIterator.range();
> +    SimpleRange range = m_underlyingIterator.range();

auto

> Source/WebCore/editing/TextIterator.cpp:1499
> +            Node& node = range.start.container;

auto

> Source/WebCore/editing/TextIterator.cpp:2508
> +    for (TextIterator iterator { createLiveRange(range).ptr(), behavior }; !iterator.atEnd(); iterator.advance()) {

I suppose the plan to also make TextIterator take SimpleRange at some point?

> Source/WebCore/editing/VisibleUnits.cpp:638
> +    Node& node = it.atEnd() ? searchRange->startContainer() : it.range().start.container.get();

auto

> Source/WebCore/editing/VisibleUnits.cpp:649
> +    return VisiblePosition(createLegacyEditingPosition(charIt.range().end), DOWNSTREAM);

Could just use { }

> Source/WebCore/editing/VisibleUnits.cpp:685
> +        SimpleRange characterRange = charIt.range();

auto

[Darin Adler]

Comment on attachment 393207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393207&action=review

Thanks very much for the review.

>> Source/WebCore/accessibility/AXObjectCache.cpp:2667
>> +    Node& node = it.atEnd() ? searchRange->startContainer() : it.range().start.container.get();
> 
> auto

You mean auto, and not auto&? For now I did auto&.

>> Source/WebCore/editing/TextIterator.cpp:1494
>> +    SimpleRange range = m_underlyingIterator.range();
> 
> auto

Done.

>> Source/WebCore/editing/TextIterator.cpp:1499
>> +            Node& node = range.start.container;
> 
> auto

Did auto& for now.

>> Source/WebCore/editing/TextIterator.cpp:2508
>> +    for (TextIterator iterator { createLiveRange(range).ptr(), behavior }; !iterator.atEnd(); iterator.advance()) {
> 
> I suppose the plan to also make TextIterator take SimpleRange at some point?

Yes, that's right. Just doing the changes one step at a time, but will get to that soon.

>> Source/WebCore/editing/VisibleUnits.cpp:638
>> +    Node& node = it.atEnd() ? searchRange->startContainer() : it.range().start.container.get();
> 
> auto

Did auto&.

>> Source/WebCore/editing/VisibleUnits.cpp:649
>> +    return VisiblePosition(createLegacyEditingPosition(charIt.range().end), DOWNSTREAM);
> 
> Could just use { }

Trying that (waiting for it to compile right now).

>> Source/WebCore/editing/VisibleUnits.cpp:685
>> +        SimpleRange characterRange = charIt.range();
> 
> auto

Done.

[Darin Adler]

Comment on attachment 393207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393207&action=review

>>> Source/WebCore/editing/TextIterator.cpp:1499
>>> +            Node& node = range.start.container;
>> 
>> auto
> 
> Did auto& for now.

Turns out this one needs to be Node&; changed it back.

[Darin Adler]

Committed r258475: <https://trac.webkit.org/changeset/258475>

[Radar WebKit Bug Importer]

<rdar://problem/60460158>

[Antti Koivisto]

> You mean auto, and not auto&? For now I did auto&.

Right, I mean "auto in a way that builds and follows WebKit coding style".

[Antti Koivisto]

...and doesn't alter the behavior.

[Darin Adler]

Consider it done, then!