stringValue() should return text() for these types of objects.
<rdar://problem/106360128>
Created attachment 465339 [details] Patch
Comment on attachment 465339 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=465339&action=review > COMMIT_MESSAGE:7 > +The implementation of the text() and stringValue() for these types of objects is fragmented between the AccessibilityNodeObject and AccessibilityRenderObject classes. This patches eliminates some duplicated code and makes it easier to determine what these methods are returning for these roles. Typo, "patches" should probably be "patch". > COMMIT_MESSAGE:8 > +As a byproduct, the patch remove ariaRoleAttributed() from the AXCoreObject interface since it is not actually needed there. Some code cleanup. Typos here. As a byproduct, the patch remove ariaRoleAttributed() Should maybe read: As a byproduct, the patch removes ariaRoleAttribute() > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2195 > + if (isPasswordField()) > + return passwordFieldValue(); I think you need to rebase — I changed this to be `isSecureField` and `secureFieldValue` in a recent patch. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2198 > + // In the former case, go first for any alt text that may have been specified. Nitpick on this wording -- feel free to take it or leave it. // In the former case, go first for any alt text that may have been specified. // In the former case, prefer any alt text that may have been specified. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2199 > + // If no alt text present, fallback to the inline static text case where textUnderElement is used. // If no alt text present // If no alt text is present
Created attachment 465352 [details] Patch
(In reply to Tyler Wilcock from comment #3) > Comment on attachment 465339 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=465339&action=review > > > COMMIT_MESSAGE:7 > > +The implementation of the text() and stringValue() for these types of objects is fragmented between the AccessibilityNodeObject and AccessibilityRenderObject classes. This patches eliminates some duplicated code and makes it easier to determine what these methods are returning for these roles. > > Typo, "patches" should probably be "patch". > > > COMMIT_MESSAGE:8 > > +As a byproduct, the patch remove ariaRoleAttributed() from the AXCoreObject interface since it is not actually needed there. Some code cleanup. > > Typos here. > > As a byproduct, the patch remove ariaRoleAttributed() > > Should maybe read: > > As a byproduct, the patch removes ariaRoleAttribute() > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2195 > > + if (isPasswordField()) > > + return passwordFieldValue(); > > I think you need to rebase — I changed this to be `isSecureField` and > `secureFieldValue` in a recent patch. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2198 > > + // In the former case, go first for any alt text that may have been specified. > > Nitpick on this wording -- feel free to take it or leave it. > > // In the former case, go first for any alt text that may have been > specified. > // In the former case, prefer any alt text that may have been specified. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2199 > > + // If no alt text present, fallback to the inline static text case where textUnderElement is used. > > // If no alt text present > // If no alt text is present All fixed. Sorry for my poor writing this morning.
Created attachment 465357 [details] Patch
Comment on attachment 465357 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=465357&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:-754 > - if (isARIAStaticText()) { do we lose any functionality by removing this block of text?
(In reply to chris fleizach from comment #7) > Comment on attachment 465357 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=465357&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:-754 > > - if (isARIAStaticText()) { > > do we lose any functionality by removing this block of text? No because: - isARIAStaticText implies isStaticText - if isStataticText then we call AccessibilityNodeObject::text which now do the right thing for both ARIA static text and actual static text.
Committed 261424@main (56bd520fb3a3): <https://commits.webkit.org/261424@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465357 [details].