Bug 223016

Summary: Release assertion failures under Editor::scanSelectionForTelephoneNumbers
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, darin, ews-watchlist, megan_gardner, mifenton, rniwa, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Tim Horton 2021-03-09 22:28:33 PST
Release assertion failures under Editor::scanSelectionForTelephoneNumbers
Comment 1 Tim Horton 2021-03-09 22:28:48 PST
Created attachment 422802 [details]
Patch
Comment 2 Tim Horton 2021-03-09 22:28:50 PST
<rdar://problem/73159921>
Comment 3 Ryosuke Niwa 2021-03-09 22:33:59 PST
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?
Comment 4 Tim Horton 2021-03-09 22:50:12 PST
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!
Comment 5 Tim Horton 2021-03-09 22:55:44 PST
Created attachment 422804 [details]
Patch
Comment 6 EWS 2021-03-10 00:19:13 PST
Committed r274203: <https://commits.webkit.org/r274203>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422804 [details].
Comment 7 Darin Adler 2021-03-10 09:51:40 PST
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.