Bug 140916 - AX: [ATK] Implement support for new AtkRole types for MathML
Summary: AX: [ATK] Implement support for new AtkRole types for MathML
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joanmarie Diggs
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-01-26 18:42 PST by Joanmarie Diggs
Modified: 2015-01-31 02:27 PST (History)
6 users (show)

See Also:


Attachments
Patch (21.82 KB, patch)
2015-01-26 18:59 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (21.78 KB, patch)
2015-01-26 21:16 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (22.50 KB, patch)
2015-01-26 22:21 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (29.44 KB, patch)
2015-01-27 15:45 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2015-01-26 18:42:09 PST
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.
Comment 1 Radar WebKit Bug Importer 2015-01-26 18:42:22 PST
<rdar://problem/19609343>
Comment 2 Joanmarie Diggs 2015-01-26 18:59:26 PST
Created attachment 245399 [details]
Patch
Comment 3 Joanmarie Diggs 2015-01-26 19:01:34 PST
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.
Comment 4 Joanmarie Diggs 2015-01-26 20:08:41 PST
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!
Comment 5 Joanmarie Diggs 2015-01-26 20:13:32 PST
Comment on attachment 245399 [details]
Patch

Clearing flag. New version coming up. Sorry for the noise.
Comment 6 Joanmarie Diggs 2015-01-26 21:16:38 PST
Created attachment 245414 [details]
Patch
Comment 7 chris fleizach 2015-01-26 21:35:21 PST
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
Comment 8 Joanmarie Diggs 2015-01-26 22:21:40 PST
Created attachment 245423 [details]
Patch
Comment 9 Joanmarie Diggs 2015-01-26 23:11:04 PST
(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.
Comment 10 Joanmarie Diggs 2015-01-27 06:21:57 PST
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.)
Comment 11 chris fleizach 2015-01-27 08:38:23 PST
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)
Comment 12 Joanmarie Diggs 2015-01-27 08:51:10 PST
(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.
Comment 13 Joanmarie Diggs 2015-01-27 08:51:38 PST
(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.
Comment 14 Joanmarie Diggs 2015-01-27 15:45:09 PST
Created attachment 245487 [details]
Patch
Comment 15 WebKit Commit Bot 2015-01-28 09:02:34 PST
Comment on attachment 245487 [details]
Patch

Clearing flags on attachment: 245487

Committed r179253: <http://trac.webkit.org/changeset/179253>
Comment 16 WebKit Commit Bot 2015-01-28 09:02:39 PST
All reviewed patches have been landed.  Closing bug.