1182469 - The mathvariant transformation is not visible in the accessible tree

Status: NEW

(Core :: Disability Access APIs, defect)

Description

[Frédéric Wang (:fredw)]

While testing Orca MathML support on Wikipedia, I noticed that special alphabets such as fraktur are not supported. Although Orca has support for math alphanum characters when explicit Unicode values are used such as in

data:text/html;charset=UTF-8,<math><mi>&#x1D504;</mi><mi>&#x1D505;</mi></math>

the equivalent construction with the mathvariant attribute (used in Wikipedia, itex2MML etc)

data:text/html;charset=UTF-8,<math><mstyle mathvariant=fraktur"><mi>A</mi><mi>B</mi></math>

does not work.

Since we are mapping the mathvariant attribute to the CSS -moz-mathvariant property, it seems to me that the most obvious way to expose mathvariant to ATs would be map -moz-mathvariant to a text attribute, just like we do for e.g. font-family in

https://dxr.mozilla.org/mozilla-central/source/accessible/base/TextAttrs.h#289

Comment 1

[alexander :surkov (:asurkov)]

Fraktur is rather a font style (https://en.wikipedia.org/wiki/Fraktur) and probably should be exposed as a font style text attribute.

Comment 2

[Frédéric Wang (:fredw)]

I don't know the difference between "font style text attribute" or just "text attribute", but I'm fine as long as screen readers can read the mathvariant...

Here is the reference on the MathML spec:

http://www.w3.org/TR/MathML/chapter3.html#presm.commatt

mathvariant: "normal" | "bold" | "italic" | "bold-italic" | "double-struck" | "bold-fraktur" | "script" | "bold-script" | "fraktur" | "sans-serif" | "bold-sans-serif" | "sans-serif-italic" | "sans-serif-bold-italic" | "monospace" | "initial" | "tailed" | "looped" | "stretched"

default: normal (except on <mi>) 

"Specifies the logical class of the token. Note that this class is more than styling, it typically conveys semantic intent; see the discussion below."

Comment 3

[alexander :surkov (:asurkov)]

each of them should be mapped to certain attributes, probably some of them are. I'd be good to check that.

Comment 4

[Frédéric Wang (:fredw)]

So the point is that MathML's mathvariant is supposed to convey semantic and so is different from CSS style. For example <mi mathvariant="bold">x</mi> is equivalent to using the Unicode character "U+1D431 x MATHEMATICAL BOLD SMALL X" but is different from x with using style font-weight=bold.

Internally, we store the mathvariant as a CSS property with an enumeration of integer values: https://dxr.mozilla.org/mozilla-central/source/layout/style/nsStyleConsts.h#605

Comment 5

[Frédéric Wang (:fredw)]

Attachment: Make nsTextFrame::GetRenderedText take mathvariant into account

Comment 6

[Frédéric Wang (:fredw)]

Attachment: Add accessibility tests for transformed text

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 8634304 [details] [diff] [review]
Add accessibility tests for transformed text

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

::: accessible/tests/mochitest/text/a11y.ini
@@ +9,5 @@
>  [test_hypertext.html]
>  [test_lineboundary.html]
>  [test_passwords.html]
>  [test_selection.html]
> +[test_transform.html]

you may put them into test_gettext.html, having transform.html for mathvariant may look a strange design

::: accessible/tests/mochitest/text/test_transform.html
@@ +121,5 @@
> +    <mi id="mathvariant2">x</mi>
> +    <mi id="mathvariant3" mathvariant="normal">x</mi>
> +    <mtext id="mathvariant4" mathvariant="bold">x</mtext>
> +    <mtext id="mathvariant5" mathvariant="italic">x</mtext>
> +    <mtext id="mathvariant6" mathvariant="bold-italic">x</mtext>

I would prefer more self-descriptive IDs like mathvariant_bolditalic

Comment 8

[Frédéric Wang (:fredw)]

Attachment: Add accessibility tests for text-transform and mathvariant

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8634303 [details] [diff] [review]
Make nsTextFrame::GetRenderedText take mathvariant into account

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

::: layout/generic/MathMLTextRunFactory.h
@@ +21,5 @@
>        mFontInflation(aFontInflation),
>        mSSTYScriptLevel(aSSTYScriptLevel) {}
>  
> +  static uint32_t MathVariant(uint32_t aCh, uint8_t aMathVar);
> +  

delete trailing whitespace

Comment 10

[Frédéric Wang (:fredw)]

Attachment: Make nsTextFrame::GetRenderedText take mathvariant into account

Comment 11

[Frédéric Wang (:fredw)]

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

Comment 12

[Frédéric Wang (:fredw)]

GetRenderedText is used only in the accessibility code and in nsTextFrame::AccessibleType:

https://dxr.mozilla.org/mozilla-central/search?q=getrenderedtext

Attachment 8634570 [details] [diff] will work in most cases, except in the two calls of HyperTextAccessible.cpp, where we want to convert between the offsets of the original and transformed strings. I get a crash when trying to navigate inside a mathvariant char with Orca.

The current conversion code relies on gfxSkipChars & gfxSkipCharsIterator which assumes that we may skip characters (e.g. whitespace) from the original string. The problem with mathvariant is that it generally transforms a BMP char to a surrogate pairs, so this increases the length of the original string. It seems to me that we'll need to rewrite a new mechanism that handles this case, so that might be a bit more involved....

@Alex: what do you think/suggest?

Comment 13

[alexander :surkov (:asurkov)]

I don't know much about GetRenderedText implementation, but it sounds ok if you make a special case for mathvariant char conversions. 

It's good idea to ask :roc for feedback.

Comment 14

[Frédéric Wang (:fredw)]

@roc: see comment 12. Any suggestion for a proper way to manage these original/transformed strings and the corresponding offset conversion, so that we can expose that to Assistive Technologies?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suggest we extend GetRenderedText to also return an array mapping offsets in the output string to offsets in the DOM string.

Comment 16

[Frédéric Wang (:fredw)]

So reading the code again, the two HyperTextAccessible functions do not actually cache the gfxSkipChars/ gfxSkipCharsIterator output parameters and only use them to map from the in-offset argument to the out-offset argument. So we probably actually do not need the skipchars classes or even an array, here. Also, aSkippedStartOffset is always zero and aSkippedMaxLength is only set to 1 to test whether the rendered string is nonempty.

So I'm suggesting to do something like

GetRenderedText(nsAString* aRenderedText, bool* aIsNonEmpty = null, bool aContentToRendered = true, uint32_t aInOffset = 0, uint32_t* aOutOffset = nullptr);

and there are three possibilities:

- if aRenderedText != nullptr, this will convert the returned the rendered text.
- if aIsNonEmpty != nullptr this will returned whether the rendered string is nonempty
- if aOutOffset != nullptr this will aInOffset to aOutOffset using aContentToRendered to decide whether it's content=>rendered or rendered=>content.

Thoughts?

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That interface looks a bit too complicated. I think it would be simpler to just output the array.

Comment 18

[Frédéric Wang (:fredw)]

Attachment: Make nsTextFrame::GetRenderedText take mathvariant into account - WIP

WIP progress patch following roc's suggestion.

This seems to return the correct offsets in general but there are some tricky cases with whitespace that would need to be debugged more carefully.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ff76b0cb8d4

Comment 19

[Frédéric Wang (:fredw)]

I'm unassigning myself until I find time to come back to this.

BTW, I forgot to say that the crash mentioned in comment 12 no longer happens and Orca correctly reads https://developer.mozilla.org/en-US/docs/Mozilla/MathML_Project/a11y#mathvariant

Comment 20

[Trevor Saunders (:tbsaunde)]

(In reply to Frédéric Wang (:fredw) from comment #18)
> Created attachment 8637242 [details] [diff] [review]
> Make nsTextFrame::GetRenderedText take mathvariant into account - WIP
> 
> WIP progress patch following roc's suggestion.
> 
> This seems to return the correct offsets in general but there are some
> tricky cases with whitespace that would need to be debugged more carefully.

yeah, whitespace is already kind of buggy.  I tried to write some tests to  check what our current behavior is.  That never got committed, but the test is still in bug 1080870.  The results are pretty strange!