Bug 232764

Summary: [iOS] Mail compose becomes unresponsive after pasting in text and attempting to type
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: akeerthi, cdumez, ews-watchlist, ggaren, megan_gardner, mifenton, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
For EWS
none
Rebase on trunk
ggaren: review+
For landing none

Wenson Hsieh
Reported 2021-11-05 13:28:40 PDT
Attachments
For EWS (16.25 KB, patch)
2021-11-05 14:11 PDT, Wenson Hsieh
no flags
Rebase on trunk (14.56 KB, patch)
2021-11-05 14:16 PDT, Wenson Hsieh
ggaren: review+
For landing (14.65 KB, patch)
2021-11-05 16:02 PDT, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2021-11-05 14:11:37 PDT Comment hidden (obsolete)
Wenson Hsieh
Comment 2 2021-11-05 14:16:24 PDT
Created attachment 443437 [details] Rebase on trunk
Geoffrey Garen
Comment 3 2021-11-05 14:54:33 PDT
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 4 2021-11-05 14:56:28 PDT
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
Comment 5 2021-11-05 15:04:13 PDT
If you self-nit then you also need to self-high-five when the patch lands :P
Wenson Hsieh
Comment 6 2021-11-05 16:02:26 PDT
Created attachment 443455 [details] For landing
Wenson Hsieh
Comment 7 2021-11-05 17:46:47 PDT
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
Comment 8 2021-11-05 17:51:23 PDT
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].
Note You need to log in before you can comment on or make changes to this bug.