Closed Bug 1108378 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: MathML, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: fredw, Assigned: fredw)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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.
Attached patch Patch (deleted) — Splinter Review
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.
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 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"
(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)
Blocks: 1110056
Assignee: nobody → fred.wang
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: