Return early when handling this notification if Accessibility is not enabled.
rdar://problem/66498397
Created attachment 405887 [details] Patch
Comment on attachment 405887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405887&action=review > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:704 > + return; unblockPreferenceServiceIfNeeded() is called in various places like in m_enhancedAccessibilityObserver. Should this check be added in unblockPreferenceServiceIfNeeded instead of in this handler? That would mirror what unblockAccessibilityServerIfNeeded is ding.
Created attachment 405918 [details] Patch
(In reply to youenn fablet from comment #3) > Comment on attachment 405887 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=405887&action=review > > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:704 > > + return; > > unblockPreferenceServiceIfNeeded() is called in various places like in > m_enhancedAccessibilityObserver. Should this check be added in > unblockPreferenceServiceIfNeeded instead of in this handler? > That would mirror what unblockAccessibilityServerIfNeeded is ding. Good catch! I added a similar check for macOS. I did keep the check in the notification handlers, since I thought it might also make sense to prevent the rest of the handler code to run if accessibility is not enabled. Thanks for reviewing!
Created attachment 405920 [details] Patch
Created attachment 405921 [details] Patch
Comment on attachment 405887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405887&action=review > Source/WebKit/ChangeLog:3 > + [iOS] Check that Accessibility is enabled when receiving the enable Accessibility notification Why?
Created attachment 405938 [details] Patch
Comment on attachment 405938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405938&action=review > Source/WebCore/PAL/ChangeLog:3 > + [iOS] Check that Accessibility is enabled when receiving the enable Accessibility notification Why? For security? Performance? Correctness? > Tools/TestWebKitAPI/Tests/WebKit/EnableAccessibility.mm:66 > + [NSApp accessibilitySetEnhancedUserInterfaceAttribute:[NSNumber numberWithBool:YES]]; I suggest @(YES) instead of [NSNumber numberWithBool:YES].
(In reply to Darin Adler from comment #10) > Comment on attachment 405938 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=405938&action=review > > > Source/WebCore/PAL/ChangeLog:3 > > + [iOS] Check that Accessibility is enabled when receiving the enable Accessibility notification > > Why? For security? Performance? Correctness? > Yes, this is for security. I will add a note to the WebKit change log. > > Tools/TestWebKitAPI/Tests/WebKit/EnableAccessibility.mm:66 > > + [NSApp accessibilitySetEnhancedUserInterfaceAttribute:[NSNumber numberWithBool:YES]]; > > I suggest @(YES) instead of [NSNumber numberWithBool:YES]. Will fix. Thanks for reviewing!
Created attachment 405942 [details] Patch
Committed r265264: <https://trac.webkit.org/changeset/265264> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405942 [details].