RESOLVED FIXED 101263
AX: MathML types need to be semantically identified in AX tree
https://bugs.webkit.org/show_bug.cgi?id=101263
Summary AX: MathML types need to be semantically identified in AX tree
chris fleizach
Reported 2012-11-05 14:58:06 PST
We need to expose the semantic MathML types through the AX tree so that screen readers can effectively output math equations. rdar://12637680
Attachments
patch (60.16 KB, patch)
2012-11-05 15:05 PST, chris fleizach
no flags
patch (60.14 KB, patch)
2012-11-05 16:20 PST, chris fleizach
webkit-ews: commit-queue-
patch (60.16 KB, patch)
2012-11-05 17:07 PST, chris fleizach
bdakin: review+
chris fleizach
Comment 1 2012-11-05 15:05:35 PST
chris fleizach
Comment 2 2012-11-05 16:20:46 PST
WebKit Review Bot
Comment 3 2012-11-05 16:23:23 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
WebKit Review Bot
Comment 4 2012-11-05 16:23:45 PST
Attachment 172432 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/rendering/mathml/RenderMathMLFenced.h:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 5 2012-11-05 16:30:45 PST
Early Warning System Bot
Comment 6 2012-11-05 16:31:45 PST
chris fleizach
Comment 7 2012-11-05 17:07:25 PST
Dominic Mazzoni
Comment 8 2012-11-05 22:56:33 PST
Comment on attachment 172447 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=172447&action=review Unofficial review looks good. > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:251 > + if (r->node()->isElementNode() && toElement(r->node())->isMathMLElement()) Should this line be in #if ENABLE(MATHML)? Have you tested compiling with and without this flag? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197 > + if (isIgnoredElementWithinMathTree()) It's unfortunate to have to add yet another method that walks the whole parent chain every time accessibilityIsIgnored is called - but we can try to improve this in another change. > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2433 > + if (isMathElement()) #if ENABLE(MATHML)
chris fleizach
Comment 9 2012-11-06 10:40:53 PST
(In reply to comment #8) > (From update of attachment 172447 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172447&action=review > > Unofficial review looks good. > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:251 > > + if (r->node()->isElementNode() && toElement(r->node())->isMathMLElement()) > > Should this line be in #if ENABLE(MATHML)? Have you tested compiling with and without this flag? > Doing so now > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1197 > > + if (isIgnoredElementWithinMathTree()) > > It's unfortunate to have to add yet another method that walks the whole parent chain every time accessibilityIsIgnored is called - but we can try to improve this in another change. https://bugs.webkit.org/show_bug.cgi?id=101370 > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2433 > > + if (isMathElement()) > > #if ENABLE(MATHML) This one is actually OK since isMathElement() is just on AXObject and returns false by default without any MathML requirements
Beth Dakin
Comment 10 2012-11-13 12:36:16 PST
Comment on attachment 172447 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=172447&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3548 > + if (node()->hasTagName(MathMLNames::munderTag) || node()->hasTagName(MathMLNames::munderoverTag)) Do you need to null-check node() here? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3563 > + if (node()->hasTagName(MathMLNames::moverTag)) Do you need to null-check node() here? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3593 > + if (node()->hasTagName(MathMLNames::msubTag) || node()->hasTagName(MathMLNames::msubsupTag)) Do you need to null-check node() here? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3608 > + if (node()->hasTagName(MathMLNames::msupTag)) Do you need to null-check node() here?
chris fleizach
Comment 11 2012-11-13 15:30:19 PST
Note You need to log in before you can comment on or make changes to this bug.