372131 - nsLinkableAccessible/nsTextAccessible shouldn't be inherited from nsHyperTextAccessbile

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

<aaronlev> I don't think nsTextAccesible should inherit from nsHyperTextAcessible
<aaronlev> because nsHyperTextAccessible is supposed to be containers of text
<aaronlev> and nsTextAccessible is for a leaf text
<aaronlev> the leaf text object is nsLinkableAccessible

Comment 1

[alexander :surkov (:asurkov)]

I didn't get why nsHTMLLabelAccessible is inherited from nsTextAccessible. I guess should be nsHyperTextAccessible. Right?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> I didn't get why nsHTMLLabelAccessible is inherited from nsTextAccessible. I
> guess should be nsHyperTextAccessible. Right?
> 

Ah, actually it may be inherited form hyper text and support actions.

Comment 3

[alexander :surkov (:asurkov)]

Aaron. Please correct me if I'm wrong. We should have
1) linkable accessible that is inherited from hyper text and supports link states and actions
2) text accessible for leafs the main propose of which is to be used in MSAA, that inherited from nsAccessibleWrap and supports link states and actions.

Right?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> (In reply to comment #1)
> > I didn't get why nsHTMLLabelAccessible is inherited from nsTextAccessible. I
> > guess should be nsHyperTextAccessible. Right?
> > 
> 
> Ah, actually it may be inherited form hyper text and support actions.
> 

nsHTMLLabelAccessible may have STATE_LINKED and STATE_TRAVERSED states if it is a child of nsLinkableAccessible. Where this usecase is from?

Comment 5

[Aaron Leventhal]

The whole reason for nsLinkableAccessible is that Windows screen readers like it if STATE_LINKED/STATE_TRAVERSED and the action methods work on the descendants of the link.

I'm not totally clear on the questions in comment 3 or comment 4, or what bug this is fixing if we do this.

Is this a cleanup that we can delay until after Firefox 3? Or does it help us now?

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)

> Is this a cleanup that we can delay until after Firefox 3? Or does it help us
> now?
> 

Right, it looks like cleanup per bug 371680 comment 3.

Comment 7

[Aaron Leventhal]

Just a note, perhaps there will be a way at some point to save memory on nsHTMLTextAccessible by removing some unneeded data members. Not important for Firefox 3, but we don't really need to cache mFirstChild, mNextSibling, mAccChildCount or relations on an nsHTMLTextAccessible. Perhaps there are other things which can be removed as well.

Comment 8

[Aaron Leventhal]

It's weird that text leafs are being used as IAccessibleText. I see it in this crash:

http://crash-stats.mozilla.com/report/index/7f4d174f-da72-11dc-b877-001a4bd43ef6

We should stop that from happening.

Comment 9

[Aaron Leventhal]

For comment 8 the safer/simpler fix for now is to just change QI instead of inheritance. I have filed bug 417518 for now.

Comment 10

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 11

[alexander :surkov (:asurkov)]

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

wrongly attached patch from bug 523789

Comment 12

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

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

r=me, nice. Makes sense to me.

Comment 14

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/3628a52e829c