Summary: | Implement AXIsolatedTreeNode::roleDescription. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andres Gonzalez <andresg_22> | ||||||
Component: | Accessibility | Assignee: | Andres Gonzalez <andresg_22> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Andres Gonzalez
2019-11-05 14:08:44 PST
Created attachment 382852 [details]
Patch
../../Source/WebCore/accessibility/AccessibilityObject.cpp: In member function ‘virtual WTF::String WebCore::AccessibilityObject::ariaLandmarkRoleDescription() const’: ../../Source/WebCore/accessibility/AccessibilityObject.cpp:2622:16: error: ‘AXARIAContentGroupText’ was not declared in this scope C:\Buildbot\WinCairo-EWS\build\Source\WebCore\accessibility/AccessibilityObject.cpp(2657): error C3861: 'AXARIAContentGroupText': identifier not found C:\Buildbot\WinCairo-EWS\build\Source\WebCore\accessibility/AccessibilityObject.cpp(2659): error C3861: 'AXARIAContentGroupText': identifier not found C:\Buildbot\WinCairo-EWS\build\Source\WebCore\accessibility/AccessibilityObject.cpp(2661): error C3861: 'AXARIAContentGroupText': identifier not found Comment on attachment 382852 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382852&action=review > Source/WebCore/ChangeLog:21 > + platforms are pending. add new line after this sentence > Source/WebCore/accessibility/AccessibilityObject.h:836 > +using PlatformRoleMap = HashMap<AccessibilityRole, String, DefaultHash<int>::Hash, WTF::UnsignedWithZeroKeyHashTraits<int>>; should this be DefaultHash <int> or unsigned? > Source/WebCore/accessibility/AccessibilityObject.h:838 > +PlatformRoleMap createPlatformRoleMap(); it doesn't seem like createPlatformRoleMap should be in the .h. It's meant to be created the first time roleToPlatformString is accessed > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:155 > + NSString* axRole = rolePlatformString(); NSString *axRole (pointer on other side) > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:264 > + __unsafe_unretained NSString *string; this file isn't ARC, so I don't think we need unsafe_unretained. that is the default behavior > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:413 > + roleMap.add(static_cast<int>(role.value), (__bridge CFStringRef)role.string); do we need this cast (__bridge CFStringRef) maybe we just need a NSString * cast do get rid of the const NSString * > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2040 > + NSString* string = m_object->rolePlatformString(); * on wrong side > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2270 > + const String roleDescription = m_object->roleDescription(); the const seems unnecessary here Created attachment 382924 [details]
Patch
(In reply to chris fleizach from comment #7) > Comment on attachment 382852 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382852&action=review > > > Source/WebCore/ChangeLog:21 > > + platforms are pending. > > add new line after this sentence Done. > > > Source/WebCore/accessibility/AccessibilityObject.h:836 > > +using PlatformRoleMap = HashMap<AccessibilityRole, String, DefaultHash<int>::Hash, WTF::UnsignedWithZeroKeyHashTraits<int>>; > > should this be DefaultHash <int> or unsigned? Changed it to unsigned. Other existing RoleMap is using int and thus I thought to leave it as is, but we probably come back and change the other too. > > > Source/WebCore/accessibility/AccessibilityObject.h:838 > > +PlatformRoleMap createPlatformRoleMap(); > > it doesn't seem like createPlatformRoleMap should be in the .h. It's meant > to be created the first time roleToPlatformString is accessed The prototype has to be visible to both the core .cpp and the platform overrides, the .mm on Mac. So the options were to duplicate the prototype or to put it in a common header. Eventually, some of these methods should be moved to their own header file. I would leave it here for now unless you feel strongly about moving it somewhere else. > > > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:155 > > + NSString* axRole = rolePlatformString(); > > NSString *axRole > (pointer on other side) Done. > > > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:264 > > + __unsafe_unretained NSString *string; > > this file isn't ARC, so I don't think we need unsafe_unretained. that is the > default behavior Done. > > > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:413 > > + roleMap.add(static_cast<int>(role.value), (__bridge CFStringRef)role.string); > > do we need this cast (__bridge CFStringRef) > maybe we just need a NSString * cast do get rid of the const NSString * Got rid of it. > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2040 > > + NSString* string = m_object->rolePlatformString(); > > * on wrong side Done. > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2270 > > + const String roleDescription = m_object->roleDescription(); > > the const seems unnecessary here Got rid of it. Thanks! Comment on attachment 382924 [details] Patch Clearing flags on attachment: 382924 Committed r252181: <https://trac.webkit.org/changeset/252181> All reviewed patches have been landed. Closing bug. |