Bug 140916 – AX: [ATK] Implement support for new AtkRole types for MathML

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Joanmarie Diggs]

ATK 2.15.4 has four new roles applicable to MathML:
* ATK_ROLE_MATH_FRACTION
* ATK_ROLE_MATH_ROOT
* ATK_ROLE_SUBSCRIPT
* ATK_ROLE_SUPERSCRIPT

We should implement support for them.

Note that in the case of subscript and superscript, the {sub,super}script functionally is not the {msub,msup} element (which includes the base). Therefore, I think what makes the most sense is to expose the script container as ATK_ROLE_SECTION (the generic textblock role) and then expose its children (the functional sub/sup) using the new roles.

[Radar WebKit Bug Importer]

<rdar://problem/19609343>

[Joanmarie Diggs]

Created attachment 245399 [details]
Patch

[Joanmarie Diggs]

NOT yet asking for review. I generated results for the Mac and am hoping they'll match for Mountain Lion and Mavericks. Tossing them at EWS to see. Ditto for the jhbuild ATK version bump for Gtk.

[Joanmarie Diggs]

Ok, NOW I'm asking for review. :)

@Chris: If you have time to review this, that would be awesome.

@Carlos: This is a heads up and request for you to bless (or deny) the jhbuild ATK version bump once I have a patch that passes review. To see what we will be able to regression test from the bump, compare the Gtk+ test changes to the Efl ones.

Thanks guys!

[Joanmarie Diggs]

Comment on attachment 245399 [details]
Patch

Clearing flag. New version coming up. Sorry for the noise.

[Joanmarie Diggs]

Created attachment 245414 [details]
Patch

[chris fleizach]

Comment on attachment 245414 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245414&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3617
> +    return this == parentObjectUnignored()->mathSuperscriptObject();

I think we should check if parentObjectUnignored() returns nullptr before calling that method on it.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3628
> +    if (!parent or !parent->isMathMultiscript())

or -> || (is 'or' being used in WebKit in other places?)

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3633
> +    for (const auto& pair : pairs) {

can you add some comments what this is trying to determine?

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3641
> +    for (const auto& pair : pairs) {

and here too

[Joanmarie Diggs]

Created attachment 245423 [details]
Patch

[Joanmarie Diggs]

(In reply to comment #7)

> I think we should check if parentObjectUnignored() returns nullptr before
> calling that method on it.

Done.
 
> or -> || (is 'or' being used in WebKit in other places?)

Done. (Was multitasking with some Python code. Apologies.)
 
> can you add some comments what this is trying to determine?

Done.

[Joanmarie Diggs]

Martin: This is a heads up and request for you to bless (or deny) the jhbuild ATK version bump once I have a patch that passes review. To see what we will be able to regression test from the bump, compare the Gtk+ test changes to the Efl ones.

(Carlos says he's ok with it, but to check with you.)

[chris fleizach]

Comment on attachment 245423 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245423&action=review

thanks!

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3642
> +    // this token is present as the second item.

With these comments, I see that I think we can probably combine these methods into one like

isMathMultiscriptObject(AccessibilityMathMultiscriptType type)

[Joanmarie Diggs]

(In reply to comment #11)
> Comment on attachment 245423 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245423&action=review
> 
> thanks!
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3642
> > +    // this token is present as the second item.
> 
> With these comments, I see that I think we can probably combine these
> methods into one like
> 
> isMathMultiscriptObject(AccessibilityMathMultiscriptType type)

Sure, but then how do I know if it's a subscript or a superscript? The goal is to distinguish them.

[Joanmarie Diggs]

(In reply to comment #12)
> (In reply to comment #11)
> > Comment on attachment 245423 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=245423&action=review
> > 
> > thanks!
> > 
> > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3642
> > > +    // this token is present as the second item.
> > 
> > With these comments, I see that I think we can probably combine these
> > methods into one like
> > 
> > isMathMultiscriptObject(AccessibilityMathMultiscriptType type)
> 
> Sure, but then how do I know if it's a subscript or a superscript? The goal
> is to distinguish them.

Never mind. Misread.

[Joanmarie Diggs]

Created attachment 245487 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 245487 [details]
Patch

Clearing flags on attachment: 245487

Committed r179253: <http://trac.webkit.org/changeset/179253>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.