Summary: | [macOS] -toggleAutomaticSpellingCorrection: menu item validation shouldn't require post-layout editor state | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | WebKit Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | akeerthi, bdakin, darin, ews-watchlist, hi, megan_gardner, mifenton, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2021-09-12 11:53:32 PDT
Created attachment 437994 [details]
For EWS
Created attachment 437995 [details]
Fix iOS build
Comment on attachment 437995 [details] Fix iOS build View in context: https://bugs.webkit.org/attachment.cgi?id=437995&action=review > Source/WebCore/editing/AlternativeTextController.cpp:348 > + } else if (RefPtr editableRoot = position.rootEditableElement()) { > + if (is<HTMLElement>(editableRoot) && !downcast<HTMLElement>(*editableRoot).shouldAutocorrect()) > + return false; > } This is null checking twice. The old code was doing that too. There are a couple different ways to avoid that. Here’s one: } else if (RefPtr editableRoot = position.rootEditableElement(); is<HTMLElement>(editableRoot)) { Here’s another: } else if (RefPtr editableRoot = position.rootEditableElement()) { if (is<HTMLElement>(*editableRoot) && !downcast<HTMLElement>(*editableRoot).shouldAutocorrect()) return false; } Comment on attachment 437995 [details] Fix iOS build View in context: https://bugs.webkit.org/attachment.cgi?id=437995&action=review Thanks for the review! >> Source/WebCore/editing/AlternativeTextController.cpp:348 >> } > > This is null checking twice. The old code was doing that too. There are a couple different ways to avoid that. Here’s one: > > } else if (RefPtr editableRoot = position.rootEditableElement(); is<HTMLElement>(editableRoot)) { > > Here’s another: > > } else if (RefPtr editableRoot = position.rootEditableElement()) { > if (is<HTMLElement>(*editableRoot) && !downcast<HTMLElement>(*editableRoot).shouldAutocorrect()) > return false; > } Good catch — I'll go with the first option since it's a bit more succinct, and also move the `!downcast<HTMLElement>(*editableRoot).shouldAutocorrect()` next to the type check. Created attachment 438075 [details]
For landing
Committed r282370 (241633@main): <https://commits.webkit.org/241633@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438075 [details]. |