Bug 223016 - Release assertion failures under Editor::scanSelectionForTelephoneNumbers
Summary: Release assertion failures under Editor::scanSelectionForTelephoneNumbers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-09 22:28 PST by Tim Horton
Modified: 2021-03-10 09:51 PST (History)
8 users (show)

See Also:


Attachments
Patch (4.41 KB, patch)
2021-03-09 22:28 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (4.45 KB, patch)
2021-03-09 22:55 PST, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.