Closed Bug 372131 Opened 14 years ago Closed 11 years ago

nsLinkableAccessible/nsTextAccessible shouldn't be inherited from nsHyperTextAccessbile

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

<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
I didn't get why nsHTMLLabelAccessible is inherited from nsTextAccessible. I guess should be nsHyperTextAccessible. Right?
Status: NEW → ASSIGNED
(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.
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?
Depends on: 376514
(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?
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?
(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.
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.
Blocks: cleana11y
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.
Keywords: access
For comment 8 the safer/simpler fix for now is to just change QI instead of inheritance. I have filed bug 417518 for now.
Depends on: 523565
Depends on: 523789
Attached patch patch (obsolete) — Splinter Review
Attachment #407714 - Flags: review?(bolterbugz)
Comment on attachment 407714 [details] [diff] [review]
patch

wrongly attached patch from bug 523789
Attachment #407714 - Attachment is obsolete: true
Attachment #407714 - Flags: review?(bolterbugz)
Attached patch patchSplinter Review
Attachment #408345 - Flags: review?(bolterbugz)
Comment on attachment 408345 [details] [diff] [review]
patch

r=me, nice. Makes sense to me.
Attachment #408345 - Flags: review?(bolterbugz) → review+
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/3628a52e829c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.