Closed Bug 1175269 Opened 9 years ago Closed 9 years ago

Add MathML roles/subroles for NSAccessibility

Categories

(Core :: Disability Access APIs, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: fredw, Assigned: fredw)

References

Details

Attachments

(3 files, 3 obsolete files)

This is the part of bug 1001641 that only deals with roles. Let's see what VoiceOver can do with that.
Summary: Add roles/subroles for NSAccessibility → Add MathML roles/subroles for NSAccessibility
Depends on: 1175164
Attached patch Patch (obsolete) — Splinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=23b07f7a76f9
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
Apple sometimes refines AXMathOperator to AXMathFenceOperator or AXMathSeparatorOperator. I'm not sure how it is used by ATs and whether it's really important to have it now, but this second version tries to get these info from the MathML layout code.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=282ebdbb198a

But actually, what I suspect Jonathan wanted to do was to create AXMathFenceOperator or AXMathSeparatorOperator accessibles for the open/close/separators of the <mfenced> element. This will probably be harder to do since the MathML rendering code only creates nsMathMLChar objects, that are not currently exposed to the outside. I'll open a separate bug for that.
Attached patch Basic testcase for mac roles (obsolete) — Splinter Review
Here is a testcase for basic MathML expressions with what I believe the role/subroles should look like, based on what I see in the WebKit code in http://trac.webkit.org/browser/trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm#L2125
@marco: Can you please check whether the roles in the accessible trees are consistent in the testcase for the two Firefox try builds VS Safari? Probably the 3 last tests may render differently and the <mfenced> case will not work well in any platforms, but I believe we can ignore these for now.

Also, it would be good to see whether that improves how VoiceOver reads MathML in Gecko, although we may not have exactly the same result as VoiceOver because the patch does not include accessible relations.
Flags: needinfo?(mzehe)
Attachment #8623938 - Attachment is obsolete: true
(In reply to Frédéric Wang (:fredw) from comment #2)
> But actually, what I suspect Jonathan wanted to do was to create
> AXMathFenceOperator or AXMathSeparatorOperator accessibles for the
> open/close/separators of the <mfenced> element. This will probably be harder
> to do since the MathML rendering code only creates nsMathMLChar objects,
> that are not currently exposed to the outside. I'll open a separate bug for
> that.

So this is bug 1175747. The test for WebKit Mac (which I think is currently broken) is http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/accessibility/mathml-elements.html

<math id="fenced">
  <mfenced open="{" close="}" separators=",,"><mi>2</mi><mi>a</mi><mi>e</mi></mfenced>
</math>

var fenced = accessibilityController.accessibleElementById("fenced").childAtIndex(0);
shouldBe("fenced.role", "'AXRole: AXGroup'");
shouldBe("fenced.subrole", "'AXSubrole: AXMathFenced'");
shouldBe("fenced.stringAttributeValue('AXMathFencedOpen')", "'{'");
shouldBe("fenced.stringAttributeValue('AXMathFencedClose')", "'}'");
var fenceValues = new Array("{", "2", ",", "a", ",", "e", "}");
for (var k = 0; k < fenceValues.length; k++) {
   var child = fenced.childAtIndex(k);
   shouldBe("child.childAtIndex(0).stringValue", "'AXValue: " + fenceValues[k] + "'");
}      

I'm not sure what will be exactly the subroles, but as I understand '{' and '}' are AXMathFenceOperator and ',' is AXMathSeparatorOperator.
I just tried the try-server build, and VoiceOver doesn't see any of the MathML stuff at all. But I don't know why that might be, because at the same time, the table itself isn't rendered to VoiceOver correctly yet because we are not exposing tables correctly yet in general. I tried the second build, for patch v2. Does it make sense at all to test the one for the first patch?
Flags: needinfo?(mzehe)
Attachment #8623445 - Flags: review?(surkov.alexander)
Comment on attachment 8623445 [details] [diff] [review]
Patch

Review of attachment 8623445 [details] [diff] [review]:
-----------------------------------------------------------------

looks good, I saw it working with relations patch applied, so rs=me
Attachment #8623445 - Flags: review?(surkov.alexander) → review+
Blocks: 1176094
Keywords: checkin-needed
Attachment #8623934 - Attachment description: Patch V2 → Patch V2 - try to get the fence/separator properties
(In reply to Marco Zehe (:MarcoZ) from comment #7)
> But I don't know why that might be, because at the same
> time, the table itself isn't rendered to VoiceOver correctly yet because we
> are not exposing tables correctly yet in general.

We also saw that bug with Alex yesterday. Is there a bug entry for that?
(In reply to Frédéric Wang (:fredw) from comment #11)
> (In reply to Marco Zehe (:MarcoZ) from comment #7)
> > But I don't know why that might be, because at the same
> > time, the table itself isn't rendered to VoiceOver correctly yet because we
> > are not exposing tables correctly yet in general.
> 
> We also saw that bug with Alex yesterday. Is there a bug entry for that?

Yes, bug 744790.
Per bug 1001641 comment 15, it's best not to expose the AXMathFence role for now.
Attachment #8623445 - Attachment is obsolete: true
Attachment #8623934 - Attachment is obsolete: true
Attachment #8624957 - Flags: review?(surkov.alexander)
Comment on attachment 8624957 [details] [diff] [review]
Patch V3 - do not expose the AXMathFence subrole

Review of attachment 8624957 [details] [diff] [review]:
-----------------------------------------------------------------

fine with me
Attachment #8624957 - Flags: review?(surkov.alexander) → review+
https://hg.mozilla.org/mozilla-central/rev/8ce4a0a6691b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: