WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
203863
Implement AXIsolatedTreeNode::roleDescription.
https://bugs.webkit.org/show_bug.cgi?id=203863
Summary
Implement AXIsolatedTreeNode::roleDescription.
Andres Gonzalez
Reported
2019-11-05 14:08:44 PST
Implement AXIsolatedTreeNode::roleDescription.
Attachments
Patch
(52.00 KB, patch)
2019-11-05 14:38 PST
,
Andres Gonzalez
no flags
Details
Formatted Diff
Diff
Patch
(61.67 KB, patch)
2019-11-06 06:49 PST
,
Andres Gonzalez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Andres Gonzalez
Comment 1
2019-11-05 14:38:45 PST
Created
attachment 382852
[details]
Patch
Radar WebKit Bug Importer
Comment 2
2019-11-05 23:24:55 PST
<
rdar://problem/56934435
>
chris fleizach
Comment 3
2019-11-05 23:25:02 PST
../../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
Radar WebKit Bug Importer
Comment 4
2019-11-05 23:25:07 PST
<
rdar://problem/56934437
>
chris fleizach
Comment 5
2019-11-05 23:25:20 PST
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
Radar WebKit Bug Importer
Comment 6
2019-11-05 23:25:32 PST
<
rdar://problem/56934443
>
chris fleizach
Comment 7
2019-11-05 23:36:04 PST
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
Andres Gonzalez
Comment 8
2019-11-06 06:49:39 PST
Created
attachment 382924
[details]
Patch
Andres Gonzalez
Comment 9
2019-11-06 07:17:12 PST
(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!
WebKit Commit Bot
Comment 10
2019-11-07 05:22:06 PST
Comment on
attachment 382924
[details]
Patch Clearing flags on attachment: 382924 Committed
r252181
: <
https://trac.webkit.org/changeset/252181
>
WebKit Commit Bot
Comment 11
2019-11-07 05:22:08 PST
All reviewed patches have been landed. Closing bug.
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