Bug 223016 – Release assertion failures under Editor::scanSelectionForTelephoneNumbers

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Release assertion failures under Editor::scanSelectionForTelephoneNumbers

[Tim Horton]

Created attachment 422802 [details]
Patch

[Tim Horton]

<rdar://problem/73159921>

[Ryosuke Niwa]

Comment on attachment 422802 [details]
Patch

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

> Source/WebCore/editing/Editor.cpp:3705
> +        if (selectedRange) {

Why not put selectedRange inside here?

> Source/WebCore/editing/Editor.cpp:3710
> +            extendedRange = extendSelection(*selectedRange, charactersToExtend);
> +        }
> +        
> +        if (extendedRange) {

Can't we just nest these if's?

[Tim Horton]

Comment on attachment 422802 [details]
Patch

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

>> Source/WebCore/editing/Editor.cpp:3705
>> +        if (selectedRange) {
> 
> Why not put selectedRange inside here?

Good point.

>> Source/WebCore/editing/Editor.cpp:3710
>> +        if (extendedRange) {
> 
> Can't we just nest these if's?

Sure, we certainly can!

[Tim Horton]

Created attachment 422804 [details]
Patch

[EWS]

Committed r274203: <https://commits.webkit.org/r274203>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422804 [details].

[Darin Adler]

Better. One other thought.

To my taste, the real problem here is calling two separate functions, isRange and firstRange, to get the same information. A better design would be to call firstRange and then check if it’s missing or *collapsed* instead of relying on a separate function call to preflight that information.