Bug 263736

<rdar://problem/117544889>

Created attachment 468359 [details] Patch

Comment on attachment 468359 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468359&action=review > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:302 > +void AXObjectCache::announce(const String& message) can we also add the iOS version of this too

Created attachment 468370 [details] Patch

(In reply to chris fleizach from comment #3) > Comment on attachment 468359 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468359&action=review > > > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:302 > > +void AXObjectCache::announce(const String& message) > > can we also add the iOS version of this too Done, thanks.

Comment on attachment 468370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468370&action=review > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); For iOS, i think what we need to do is postPlatformNotification(object, AXAnnouncement); then add AXAnnouncement to enum AXNotification then we need to update ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification notification) { to add the enum to string mapping

(In reply to chris fleizach from comment #6) > Comment on attachment 468370 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468370&action=review > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); > > For iOS, i think what we need to do is > > postPlatformNotification(object, AXAnnouncement); > > then add > > AXAnnouncement to > enum AXNotification > > > then we need to update > > ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification > notification) > { > > to add the enum to string mapping This is intended for a message that is not associated with any object, so postPlatformNotification is not the path here. I'll change the AXNotification to platform notification name mapping.

(In reply to Andres Gonzalez from comment #7) > (In reply to chris fleizach from comment #6) > > Comment on attachment 468370 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=468370&action=review > > > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > > > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); > > > > For iOS, i think what we need to do is > > > > postPlatformNotification(object, AXAnnouncement); > > > > then add > > > > AXAnnouncement to > > enum AXNotification > > > > > > then we need to update > > > > ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification > > notification) > > { > > > > to add the enum to string mapping > > This is intended for a message that is not associated with any object, so > postPlatformNotification is not the path here. I'll change the > AXNotification to platform notification name mapping. interestingly, most of these notifications aren't associated with the user, but we need to pass these through the postPlatformNotification so that the webkit bundle can process the notifications correctly

(In reply to chris fleizach from comment #8) > (In reply to Andres Gonzalez from comment #7) > > (In reply to chris fleizach from comment #6) > > > Comment on attachment 468370 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=468370&action=review > > > > > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > > > > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); > > > > > > For iOS, i think what we need to do is > > > > > > postPlatformNotification(object, AXAnnouncement); > > > > > > then add > > > > > > AXAnnouncement to > > > enum AXNotification > > > > > > > > > then we need to update > > > > > > ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification > > > notification) > > > { > > > > > > to add the enum to string mapping > > > > This is intended for a message that is not associated with any object, so > > postPlatformNotification is not the path here. I'll change the > > AXNotification to platform notification name mapping. > > interestingly, most of these notifications aren't associated with the user, > but we need to pass these through the postPlatformNotification so that the > webkit bundle can process the notifications correctly To go that path, we then need to modify - (void)accessibilityOverrideProcessNotification:(NSString *)notificationName { // This is overridden by the Accessibility system to post-process notifications. // When it is done, it will call back into handleNotificationRelayToChrome. } To take an additional NSString *message. Is that what you are proposing?

(In reply to Andres Gonzalez from comment #9) > (In reply to chris fleizach from comment #8) > > (In reply to Andres Gonzalez from comment #7) > > > (In reply to chris fleizach from comment #6) > > > > Comment on attachment 468370 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=468370&action=review > > > > > > > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > > > > > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); > > > > > > > > For iOS, i think what we need to do is > > > > > > > > postPlatformNotification(object, AXAnnouncement); > > > > > > > > then add > > > > > > > > AXAnnouncement to > > > > enum AXNotification > > > > > > > > > > > > then we need to update > > > > > > > > ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification > > > > notification) > > > > { > > > > > > > > to add the enum to string mapping > > > > > > This is intended for a message that is not associated with any object, so > > > postPlatformNotification is not the path here. I'll change the > > > AXNotification to platform notification name mapping. > > > > interestingly, most of these notifications aren't associated with the user, > > but we need to pass these through the postPlatformNotification so that the > > webkit bundle can process the notifications correctly > > To go that path, we then need to modify > > - (void)accessibilityOverrideProcessNotification:(NSString *)notificationName > { > // This is overridden by the Accessibility system to post-process > notifications. > // When it is done, it will call back into > handleNotificationRelayToChrome. > } > > To take an additional NSString *message. Is that what you are proposing? yea we will need to do that.

Created attachment 468413 [details] Patch

(In reply to chris fleizach from comment #10) > (In reply to Andres Gonzalez from comment #9) > > (In reply to chris fleizach from comment #8) > > > (In reply to Andres Gonzalez from comment #7) > > > > (In reply to chris fleizach from comment #6) > > > > > Comment on attachment 468370 [details] > > > > > Patch > > > > > > > > > > View in context: > > > > > https://bugs.webkit.org/attachment.cgi?id=468370&action=review > > > > > > > > > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:47 > > > > > > + relayNotification(UIAccessibilityAnnouncementNotification, [static_cast<NSString *>(message) dataUsingEncoding:NSUTF8StringEncoding]); > > > > > > > > > > For iOS, i think what we need to do is > > > > > > > > > > postPlatformNotification(object, AXAnnouncement); > > > > > > > > > > then add > > > > > > > > > > AXAnnouncement to > > > > > enum AXNotification > > > > > > > > > > > > > > > then we need to update > > > > > > > > > > ASCIILiteral AXObjectCache::notificationPlatformName(AXNotification > > > > > notification) > > > > > { > > > > > > > > > > to add the enum to string mapping > > > > > > > > This is intended for a message that is not associated with any object, so > > > > postPlatformNotification is not the path here. I'll change the > > > > AXNotification to platform notification name mapping. > > > > > > interestingly, most of these notifications aren't associated with the user, > > > but we need to pass these through the postPlatformNotification so that the > > > webkit bundle can process the notifications correctly > > > > To go that path, we then need to modify > > > > - (void)accessibilityOverrideProcessNotification:(NSString *)notificationName > > { > > // This is overridden by the Accessibility system to post-process > > notifications. > > // When it is done, it will call back into > > handleNotificationRelayToChrome. > > } > > > > To take an additional NSString *message. Is that what you are proposing? > > yea we will need to do that. Done.

Comment on attachment 468413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468413&action=review > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:132 > + [rootObject()->wrapper() accessibilityPostedNotification:notificationName.get() userInfo:@{ notificationName.get() : nsMessage }]; this has to be changed somewhere as well. > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:304 > + NSAccessibilityPostNotificationWithUserInfo(NSApp, NSAccessibilityAnnouncementRequestedNotification, @{ seems possible to add a test for this behavior.

Created attachment 468426 [details] Patch

(In reply to chris fleizach from comment #13) > Comment on attachment 468413 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468413&action=review > > > Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm:132 > > + [rootObject()->wrapper() accessibilityPostedNotification:notificationName.get() userInfo:@{ notificationName.get() : nsMessage }]; > > this has to be changed somewhere as well. > > > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:304 > > + NSAccessibilityPostNotificationWithUserInfo(NSApp, NSAccessibilityAnnouncementRequestedNotification, @{ > > seems possible to add a test for this behavior. Added test and consolidated iOS and MacOS implementations.

Created attachment 468428 [details] Patch

Comment on attachment 468426 [details] Patch build error ERROR: symbol __ZN7WebCore13AXObjectCache8announceERKN3WTF6StringE Command PhaseScriptExecution failed with a nonzero exit code

Comment on attachment 468426 [details] Patch build error ERROR: symbol __ZN7WebCore13AXObjectCache8announceERKN3WTF6StringE Command PhaseScriptExecution failed with a nonzero exit code

build error ERROR: symbol __ZN7WebCore13AXObjectCache8announceERKN3WTF6StringE Command PhaseScriptExecution failed with a nonzero exit code

Created attachment 468431 [details] Patch

Created attachment 468441 [details] Patch Excluding test from platforms that don't implement announce.

Created attachment 468445 [details] Patch

Committed 270135@main (4f1f74a1f3c8): <https://commits.webkit.org/270135@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468445 [details].