Bug 232764 – [iOS] Mail compose becomes unresponsive after pasting in text and attempting to type

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

[Wenson Hsieh]

rdar://84669661

[Wenson Hsieh]

Created attachment 443436 [details]
For EWS

[Wenson Hsieh]

Created attachment 443437 [details]
Rebase on trunk

[Geoffrey Garen]

Comment on attachment 443437 [details]
Rebase on trunk

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

r=me

> Source/WebCore/editing/VisibleUnits.cpp:1511
> +    case TextGranularity::SentenceGranularity: {
> +        auto boundaryInDirection = useDownstream ? endOfSentence : startOfSentence;
> +        boundary = vp == boundaryInDirection(vp) ? vp : boundaryInDirection(useDownstream ? previousSentencePosition(vp) : nextSentencePosition(vp));
>          break;
> +    }

To my taste, this would read better as 

If (vp == boundaryInDirection(vp)) {
    boundary = vp;
    break;
}

auto position = useDownstream ? previousSentencePosition(vp) : nextSentencePosition(vp);
boundary = boundaryInDirection(position);
break;

More granular to step in a debugger and diff in a source control tool this way too.

[Wenson Hsieh]

Comment on attachment 443437 [details]
Rebase on trunk

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

>> Source/WebCore/editing/VisibleUnits.cpp:1511
>> +    }
> 
> To my taste, this would read better as 
> 
> If (vp == boundaryInDirection(vp)) {
>     boundary = vp;
>     break;
> }
> 
> auto position = useDownstream ? previousSentencePosition(vp) : nextSentencePosition(vp);
> boundary = boundaryInDirection(position);
> break;
> 
> More granular to step in a debugger and diff in a source control tool this way too.

Sounds good! Will change to that.

> Tools/ChangeLog:9
> +        Add a layout test that exercises the hang (and also verifies that we don't hit any debug assertions in the

(self-nit: s/layout test/API test/)

[Geoffrey Garen]

If you self-nit then you also need to self-high-five when the patch lands :P

[Wenson Hsieh]

Created attachment 443455 [details]
For landing

[Wenson Hsieh]

Comment on attachment 443455 [details]
For landing

(In reply to Geoffrey Garen from comment #5)
> If you self-nit then you also need to self-high-five when the patch lands :P

👏

[EWS]

Committed r285359 (243918@main): <https://commits.webkit.org/243918@main>

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