nsLinkableAccessible/nsTextAccessible shouldn't be inherited from nsHyperTextAccessbile

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 2 bugs, {access})

Trunk
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

Comment 1

12 years ago
I didn't get why nsHTMLLabelAccessible is inherited from nsTextAccessible. I guess should be nsHyperTextAccessible. Right?
Status: NEW → ASSIGNED
(Assignee)

Comment 2

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

Comment 3

12 years ago
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?
(Assignee)

Updated

12 years ago
Depends on: 376514
(Assignee)

Comment 4

12 years ago
(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

12 years ago
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?
(Assignee)

Comment 6

12 years ago
(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

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

Updated

11 years ago
Blocks: 389800

Comment 8

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

Updated

11 years ago
Blocks: 368895, 396346
Keywords: access

Comment 9

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

Updated

9 years ago
Depends on: 523565
(Assignee)

Updated

9 years ago
Depends on: 523789
(Assignee)

Comment 10

9 years ago
Created attachment 407714 [details] [diff] [review]
patch
Attachment #407714 - Flags: review?(bolterbugz)
(Assignee)

Comment 11

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

Comment 12

9 years ago
Created attachment 408345 [details] [diff] [review]
patch
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+
(Assignee)

Comment 14

9 years ago
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/3628a52e829c
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.