917598 - ISimpleDOMNode::innerHTML does not work on math elements

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

In the absence of bug 916419 (and because some tools consume MathML directly), I was investigating prototyping math access in NVDA using ISimpleDOMNode::innerHTML. Unfortunately, it looks like it returns NULL when called on math elements. If you call it on the parent node, it does include the math element, but that's not desirable when the math element has siblings.

Str:
1. Open https://eyeasme.com/Joe/MathML/MathML_browser_test
2. Locate any of the math element (role equation) accessibles.
3. Call ISimpleDOMNode::innerHTML.
Expected: The content of the math element should be returned.
Actual: NULL.
4. Get the parent using ISimpleDOMNode::parentNode.
5. On the parent, call ISimpleDOMNode::innerHTML.
Result (correct): The markup of the math element is included in the returned value.

Comment 1

[James Teh [:Jamie]]

Note that calling innerHTML on math elements using the Web Console works as expected.

Comment 2

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #0)
> In the absence of bug 916419 (and because some tools consume MathML
> directly), I was investigating prototyping math access in NVDA using
> ISimpleDOMNode::innerHTML. 

interesting idea, basically that's all you need to make MathML accessible


(In reply to James Teh [:Jamie] from comment #1)
> Note that calling innerHTML on math elements using the Web Console works as
> expected.

it must be a plain fix then, I'll file a patch

Comment 3

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #2)
> (In reply to James Teh [:Jamie] from comment #0)
> > In the absence of bug 916419 (and because some tools consume MathML
> > directly), I was investigating prototyping math access in NVDA using
> > ISimpleDOMNode::innerHTML. 
> 
> interesting idea, basically that's all you need to make MathML accessible

to read it only of course

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 5

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 6

[Trevor Saunders (:tbsaunde)]

I thought I commented here before but apparently not :(  I'm not convinced this is something I want to fix mostly because I don't want to  encourage people to use ISimpleDOM

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #6)
> I thought I commented here before but apparently not :(  I'm not convinced
> this is something I want to fix mostly because I don't want to  encourage
> people to use ISimpleDOM

innerHTML seems to be a right approach because that's the format the math processing libraries relies on. If we start to expose an accessible tree plus innherHTML then it seems that's all AT would need. We don't encourage AT to use ISimpleDOM but then we should provide innerHTML otherwise, for example, via object attributes (probably via IA2 1.3 enhanced version only to keep it performant)

Comment 8

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #7)
> (In reply to Trevor Saunders (:tbsaunde) from comment #6)
> > I thought I commented here before but apparently not :(  I'm not convinced
> > this is something I want to fix mostly because I don't want to  encourage
> > people to use ISimpleDOM
> 
> innerHTML seems to be a right approach because that's the format the math
> processing libraries relies on. If we start to expose an accessible tree
> plus innherHTML then it seems that's all AT would need. We don't encourage
> AT to use ISimpleDOM but then we should provide innerHTML otherwise, for

I think if we fix this then you'll end up encouraging people to use ISimpleDOM whether or not you say that.  If you want to expose innerhtml that would probably fine, just don't do it with ISimpleDOM.

> example, via object attributes (probably via IA2 1.3 enhanced version only
> to keep it performant)

I'd expect its probably fine to just expose innerHTML if content->IsMathml() perf wise, atleast until people stop calling GetAttributes()

Comment 9

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> (In reply to alexander :surkov from comment #7)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #6)
> > > I thought I commented here before but apparently not :(  I'm not convinced
> > > this is something I want to fix mostly because I don't want to  encourage
> > > people to use ISimpleDOM
> > 
> > innerHTML seems to be a right approach because that's the format the math
> > processing libraries relies on. If we start to expose an accessible tree
> > plus innherHTML then it seems that's all AT would need. We don't encourage
> > AT to use ISimpleDOM but then we should provide innerHTML otherwise, for
> 
> I think if we fix this then you'll end up encouraging people to use
> ISimpleDOM whether or not you say that.

> If you want to expose innerhtml
> that would probably fine, just don't do it with ISimpleDOM.

I let them do that :) Anyway it sounds like a valid bug regardless of consequences it might have. Let's just provide IA2 alternative the same time when we do this fix.

> > example, via object attributes (probably via IA2 1.3 enhanced version only
> > to keep it performant)
> 
> I'd expect its probably fine to just expose innerHTML if content->IsMathml()
> perf wise, atleast until people stop calling GetAttributes()

perhaps that's ok. Let's get Jamie's opinion.

Comment 10

[James Teh [:Jamie]]

The question is why you're discouraging use of ISimpleDOM. My understanding is that you discourage its use for general access to the web in favour of accessibility APIs (and I agree). However, if you want to retrieve markup, that is a very DOM-ish thing to do, so it makes sense for it to be done with a DOM-ish API.

Arguably, it doesn't make sense for markup to be exposed via accessibility APIs. Yes, tools for presenting math often do take MathML, but you could argue that there might be tools for presenting web content which take HTML. If ATs are fetching math via accessibility APIs, maybe it's their responsibility to format it back into the appropriate MathML. I'm still not certain on these issues yet.

My argument here is that this is a DOM API and being able to fetch innerHTML for any node in the DOM makes sense for a DOM API. If you really don't want people using this API at all (but see my first point above), then I guess it makes sense to wontfix this.

Comment 11

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #10)
> The question is why you're discouraging use of ISimpleDOM. My understanding
> is that you discourage its use for general access to the web in favour of
> accessibility APIs (and I agree). However, if you want to retrieve markup,
> that is a very DOM-ish thing to do, so it makes sense for it to be done with
> a DOM-ish API.
> 
> Arguably, it doesn't make sense for markup to be exposed via accessibility
> APIs. Yes, tools for presenting math often do take MathML, but you could
> argue that there might be tools for presenting web content which take HTML.
> If ATs are fetching math via accessibility APIs, maybe it's their
> responsibility to format it back into the appropriate MathML. I'm still not
> certain on these issues yet.

I share this point. One thing is ISimpleDOMNode is not standard, that's why I was thinking about IA2 approach but probably IA2 should be kept free from DOM stuff.

Comment 12

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #11)
> (In reply to James Teh [:Jamie] from comment #10)
> > The question is why you're discouraging use of ISimpleDOM. My understanding
> > is that you discourage its use for general access to the web in favour of
> > accessibility APIs (and I agree). However, if you want to retrieve markup,
> > that is a very DOM-ish thing to do, so it makes sense for it to be done with
> > a DOM-ish API.
> > 
> > Arguably, it doesn't make sense for markup to be exposed via accessibility
> > APIs. Yes, tools for presenting math often do take MathML, but you could
> > argue that there might be tools for presenting web content which take HTML.
> > If ATs are fetching math via accessibility APIs, maybe it's their
> > responsibility to format it back into the appropriate MathML. I'm still not
> > certain on these issues yet.
> 
> I share this point. One thing is ISimpleDOMNode is not standard, that's why
> I was thinking about IA2 approach but probably IA2 should be kept free from
> DOM stuff.

Well, it already has html attributes tag xml-roles for example.

Comment 13

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #9)
> (In reply to Trevor Saunders (:tbsaunde) from comment #8)
> > (In reply to alexander :surkov from comment #7)
> > > (In reply to Trevor Saunders (:tbsaunde) from comment #6)
> > > > I thought I commented here before but apparently not :(  I'm not convinced
> > > > this is something I want to fix mostly because I don't want to  encourage
> > > > people to use ISimpleDOM
> > > 
> > > innerHTML seems to be a right approach because that's the format the math
> > > processing libraries relies on. If we start to expose an accessible tree
> > > plus innherHTML then it seems that's all AT would need. We don't encourage
> > > AT to use ISimpleDOM but then we should provide innerHTML otherwise, for
> > 
> > I think if we fix this then you'll end up encouraging people to use
> > ISimpleDOM whether or not you say that.
> 
> > If you want to expose innerhtml
> > that would probably fine, just don't do it with ISimpleDOM.
> 
> I let them do that :) Anyway it sounds like a valid bug regardless of
> consequences it might have. Let's just provide IA2 alternative the same time
> when we do this fix.

saying that the spec for ISimpleDOM is the code and such it definitionally can't have bugs other than crashes is tempting but if you really like I guess we can fix things about it that don't make sense

Comment 14

[James Teh [:Jamie]]

(In reply to Trevor Saunders (:tbsaunde) from comment #12)
> Well, it already has html attributes tag xml-roles for example.
That argument doesn't quite hold, since xml-roles actually gets massaged by Gecko for HTML 5 structural elements. I argued a long time ago that xml-roles should have been called "landmarks" and excluded non-landmark roles, but I lost that battle.

Trev, if you're in strong support of exposing markup, why are you so strongly in favour of completely deprecating ISimpleDOM? These two things seem incompatible to me. I'd understand more if you were against exposing markup at all, but that's not the argument you're making.

Comment 15

[Trevor Saunders (:tbsaunde)]

(In reply to James Teh [:Jamie] from comment #14)
> (In reply to Trevor Saunders (:tbsaunde) from comment #12)
> > Well, it already has html attributes tag xml-roles for example.
> That argument doesn't quite hold, since xml-roles actually gets massaged by
> Gecko for HTML 5 structural elements. I argued a long time ago that
> xml-roles should have been called "landmarks" and excluded non-landmark
> roles, but I lost that battle.

sure, there's still tag, and I'm fairly sure I could find other stuff if I checked a little (css stuff in attributes? text attributes we copy from css?)

> Trev, if you're in strong support of exposing markup, why are you so

am I? I wasn't aware of that,  I think it may be a convienent hack, and practically it may just be the simplest solution, but feels a bit like a layering violation.

> strongly in favour of completely deprecating ISimpleDOM? These two things
> seem incompatible to me. I'd understand more if you were against exposing
> markup at all, but that's not the argument you're making.

I think my main argument against ISimpleDOM would be that it does what it does kind of badly, and causes me grief.

Comment 16

[James Teh [:Jamie]]

(In reply to Trevor Saunders (:tbsaunde) from comment #15)
> > Trev, if you're in strong support of exposing markup, why are you so
> am I? I wasn't aware of that,  I think it may be a convienent hack
Ah. My apologies for the misunderstanding. Yeah, convenient hack is how i feel about it, too. I guess I just feel ISimpleDOM is a more appropriate place for it than IA2 (particularly as we might get a better way for IA2 in bug 916419), but again, it's not my decision.

Comment 17

[alexander :surkov (:asurkov)]

where we are at this point?

Comment 18

[James Teh [:Jamie]]

In my prototype, I'm now crawling the DOM rather than using innerHTML, since i needed a hierarchy anyway. So, I don't need this any more, at least for now. That said, it might be useful to someone some day and if there's a patch waiting, it seems sad to let it go to waste.

Comment 19

[alexander :surkov (:asurkov)]

Trev, can you summarize why you think we shouldn't fix bug (if you think so :) )

My point is innerHTML should work on any ISimpleDOMNode since ISimpleDOMNode is DOM oriented rather than HTML specific.

Comment 20

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #19)
> Trev, can you summarize why you think we shouldn't fix bug (if you think so
> :) )

Stuff that's broken about ISimpleDOM will hopefully encourage people to stop using it.

> My point is innerHTML should work on any ISimpleDOMNode since ISimpleDOMNode
> is DOM oriented rather than HTML specific.

Well, arguably its its .InnerHTML not .InnerSource, but given we ignore that internally that's kind of a silly argument.

Comment 21

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #20)
> (In reply to alexander :surkov from comment #19)
> > Trev, can you summarize why you think we shouldn't fix bug (if you think so
> > :) )
> 
> Stuff that's broken about ISimpleDOM will hopefully encourage people to stop
> using it.

The end *does not* justify the means :) we need to help people rather bring them inconveniences.

> > My point is innerHTML should work on any ISimpleDOMNode since ISimpleDOMNode
> > is DOM oriented rather than HTML specific.
> 
> Well, arguably its its .InnerHTML not .InnerSource, but given we ignore that
> internally that's kind of a silly argument.

just naming, consider that HTML thing as xerox or pampers, dom::Element::innerHTML works on mathml which strictly speaking is not from HTML namespace.

anyway, are you sure we should not have a fix here or are you flexible?

Comment 22

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #21)
> (In reply to Trevor Saunders (:tbsaunde) from comment #20)
> > (In reply to alexander :surkov from comment #19)
> > > Trev, can you summarize why you think we shouldn't fix bug (if you think so
> > > :) )
> > 
> > Stuff that's broken about ISimpleDOM will hopefully encourage people to stop
> > using it.
> 
> The end *does not* justify the means :) we need to help people rather bring
> them inconveniences.

probably disagree, it was there choice that gave them inconveniences.

> anyway, are you sure we should not have a fix here or are you flexible?

I guess fixing this wouldn't be too terrible, but why would we want to?

Comment 23

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #22)

> > anyway, are you sure we should not have a fix here or are you flexible?
> 
> I guess fixing this wouldn't be too terrible, but why would we want to?

Practically it allows screen readers to feed mathml to libraries for speech, so if they have an accessible tree then they can get mathml support almost for free. At least this seems to be a great step for the start. Having something working right now will let us see clearer what we want achieve. Like would it be full-pledged API to expose mathml or just new few relations to improve navigation.

Comment 24

[Trevor Saunders (:tbsaunde)]

Comment on attachment 806700 [details] [diff] [review]
patch

Well, I'm still not really a fan of fixing this, but I guess getting rid of usage of nsIDOMHTMLelement is useful...

Comment 25

[alexander :surkov (:asurkov)]

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

Comment 26

[Carsten Book [:Tomcat]]

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