The mathvariant transformation is not visible in the accessible tree

NEW
Unassigned

Status

()

3 years ago
3 years ago

People

(Reporter: fredw, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox42 affected)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
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
Fraktur is rather a font style (https://en.wikipedia.org/wiki/Fraktur) and probably should be exposed as a font style text attribute.
(Reporter)

Comment 2

3 years ago
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."
each of them should be mapped to certain attributes, probably some of them are. I'd be good to check that.
(Reporter)

Comment 4

3 years ago
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
(Reporter)

Updated

3 years ago
Summary: Expose -moz-math-variant as a text attribute → The mathvariant transformation is not visible in the accessible tree
(Reporter)

Comment 5

3 years ago
Created attachment 8634303 [details] [diff] [review]
Make nsTextFrame::GetRenderedText take mathvariant into account
Attachment #8634303 - Flags: review?(roc)
(Reporter)

Comment 6

3 years ago
Created attachment 8634304 [details] [diff] [review]
Add accessibility tests for transformed text
Attachment #8634304 - Flags: review?(surkov.alexander)
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
Attachment #8634304 - Flags: review?(surkov.alexander) → review+
(Reporter)

Comment 8

3 years ago
Created attachment 8634410 [details] [diff] [review]
Add accessibility tests for text-transform and mathvariant
Attachment #8634304 - Attachment is obsolete: true
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
Attachment #8634303 - Flags: review?(roc) → review+
(Reporter)

Comment 10

3 years ago
Created attachment 8634570 [details] [diff] [review]
Make nsTextFrame::GetRenderedText take mathvariant into account
Attachment #8634303 - Attachment is obsolete: true
(Reporter)

Updated

3 years ago
Keywords: checkin-needed
(Reporter)

Comment 12

3 years ago
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?
Flags: needinfo?(surkov.alexander)
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.
Flags: needinfo?(surkov.alexander)
(Reporter)

Comment 14

3 years ago
@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?
Flags: needinfo?(roc)
I suggest we extend GetRenderedText to also return an array mapping offsets in the output string to offsets in the DOM string.
Flags: needinfo?(roc)
(Reporter)

Comment 16

3 years ago
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?
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(roc)
That interface looks a bit too complicated. I think it would be simpler to just output the array.
Flags: needinfo?(roc)
(Reporter)

Updated

3 years ago
Flags: needinfo?(surkov.alexander)
(Reporter)

Comment 18

3 years ago
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.

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

Comment 19

3 years ago
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
Assignee: fred.wang → nobody
(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!
You need to log in before you can comment on or make changes to this bug.