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

RESOLVED FIXED in mozilla37

Status

()

Core
MathML
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: fredw, Assigned: fredw)

Tracking

(Blocks: 1 bug)

Trunk
mozilla37
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

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

Comment 1

3 years ago
Created attachment 8532916 [details] [diff] [review]
Patch
(Assignee)

Comment 2

3 years ago
Comment on attachment 8532916 [details] [diff] [review]
Patch

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=813054048f8c
Attachment #8532916 - Flags: review?(karlt)

Comment 3

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

Updated

3 years ago
Blocks: 916419

Comment 4

3 years ago
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.
Attachment #8532916 - Flags: review?(karlt) → review+

Comment 5

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

Comment 6

3 years ago
(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?
Blocks: 1001634
Flags: needinfo?(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.
Flags: needinfo?(karlt)
(Assignee)

Updated

3 years ago
Blocks: 1110056
(Assignee)

Comment 8

3 years ago
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=813054048f8c
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Assignee: nobody → fred.wang
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec1980757e6a
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ec1980757e6a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.