rdar://84669661
Created attachment 443436 [details] For EWS
Created attachment 443437 [details] Rebase on trunk
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.
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/)
If you self-nit then you also need to self-high-five when the patch lands :P
Created attachment 443455 [details] For landing
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 👏
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].