WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
230205
[macOS] -toggleAutomaticSpellingCorrection: menu item validation shouldn't require post-layout editor state
https://bugs.webkit.org/show_bug.cgi?id=230205
Summary
[macOS] -toggleAutomaticSpellingCorrection: menu item validation shouldn't re...
Wenson Hsieh
Reported
2021-09-12 11:53:32 PDT
.
Attachments
For EWS
(17.70 KB, patch)
2021-09-12 12:40 PDT
,
Wenson Hsieh
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Fix iOS build
(17.21 KB, patch)
2021-09-12 13:04 PDT
,
Wenson Hsieh
darin
: review+
Details
Formatted Diff
Diff
For landing
(17.25 KB, patch)
2021-09-13 13:56 PDT
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Wenson Hsieh
Comment 1
2021-09-12 12:40:15 PDT
Created
attachment 437994
[details]
For EWS
Wenson Hsieh
Comment 2
2021-09-12 13:04:55 PDT
Created
attachment 437995
[details]
Fix iOS build
Darin Adler
Comment 3
2021-09-13 13:28:12 PDT
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; }
Wenson Hsieh
Comment 4
2021-09-13 13:37:13 PDT
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.
Wenson Hsieh
Comment 5
2021-09-13 13:56:57 PDT
Created
attachment 438075
[details]
For landing
EWS
Comment 6
2021-09-13 15:49:16 PDT
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]
.
Radar WebKit Bug Importer
Comment 7
2021-09-13 15:50:16 PDT
<
rdar://problem/83076301
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug