1108378 - Do not expose the <mphantom> element to the accessible tree

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

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

The <mphantom> element is not relevant for accessibility purpose and should not be exposed.

I guess the easiest fix is to do as in WebKit and add a rule

mphantom {
  visibility: hidden;
}

into the user agent stylesheet.

Comment 1

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

Attachment: Patch

Comment 2

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

Comment on attachment 8532916 [details] [diff] [review]
Patch

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=813054048f8c

Comment 3

[alexander :surkov (:asurkov)]

I'm not sure if mphantom might be useful for AT to detect boundaries of its area or something. per bug 1001634 comment #15 Joanie doesn't see the need. I'm curious what Jamie thinks.

Comment 4

[James Teh [:Jamie]]

I don't think this is useful either, but my knowledge of math is extremely minimal. It might be useful to detect spacing during navigation, but you can do that by comparing the locations of two nodes anyway.

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 8532916 [details] [diff] [review]
Patch

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

::: layout/mathml/mathml.css
@@ +328,5 @@
>  */
> +
> +/* Do not expose the <mphantom> element to the accessible tree. */
> +mphantom {
> +    visibility: hidden;

regarding the approach, it clearly looks like a hack because the style was introduced for accessibility only, i.e. layout doesn't need it, it works it out somehow else. So ideally either layout should rely on this style too or accessibility should know something more about layout business.

If you don't want go deep, then at least I would change the comment to something like "mphantom element is not visible, but still takes space on the page and not exposed to accessibility"

Comment 6

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

(In reply to alexander :surkov from comment #5)
> regarding the approach, it clearly looks like a hack because the style was
> introduced for accessibility only, i.e. layout doesn't need it, it works it
> out somehow else. So ideally either layout should rely on this style too or
> accessibility should know something more about layout business.
> 
> If you don't want go deep, then at least I would change the comment to
> something like "mphantom element is not visible, but still takes space on
> the page and not exposed to accessibility"

For layout, I believe we could just implement the <mphantom> element using "visibility: hidden" too and use the nsMathMLmrowFrame class instead of a specific nsMathMLmphantom class. This is essentially how it is done in WebKit (the only thing that needed some care was with drawing of e.g. fraction or square root bars that should only happen when the CSS visibility is != hidden). This could be tried in a follow-up patch, though. @Karl: do you know why <mphantom> was implemented with its own nsMathMLmphantom?

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to Frédéric Wang (:fredw) from comment #6)
> For layout, I believe we could just implement the <mphantom> element using
> "visibility: hidden" too and use the nsMathMLmrowFrame class instead of a
> specific nsMathMLmphantom class. This is essentially how it is done in
> WebKit (the only thing that needed some care was with drawing of e.g.
> fraction or square root bars that should only happen when the CSS visibility
> is != hidden). This could be tried in a follow-up patch, though. @Karl: do
> you know why <mphantom> was implemented with its own nsMathMLmphantom?

I guess nsMathMLmphantom exists solely for the empty BuildDisplayList() override, which would no longer be necessary with visibility:hidden.
nsMathMLmrowFrame sounds good to me.

Comment 8

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=813054048f8c

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec1980757e6a

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ec1980757e6a