Open Bug 1182469 Opened 9 years ago Updated 2 years ago

The mathvariant transformation is not visible in the accessible tree


(Core :: Disability Access APIs, defect)




Tracking Status
firefox42 --- affected


(Reporter: fredw, Unassigned)




(3 files, 2 obsolete files)

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


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
Fraktur is rather a font style ( and probably should be exposed as a font style text attribute.
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:

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.
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:
Summary: Expose -moz-math-variant as a text attribute → The mathvariant transformation is not visible in the accessible tree
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+
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+
Keywords: checkin-needed
GetRenderedText is used only in the accessibility code and in nsTextFrame::AccessibleType:

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)
@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)
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.

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)
Flags: needinfo?(surkov.alexander)
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.
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
Assignee: → 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!
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.