157485 - Implement nsIAccessibleText for nsHTMLTextFieldAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Kyle Yuan]

Every object that support nsIAccessibleEditableText must support 
nsIAccessibleText too.

Comment 1

[Kyle Yuan]

Attachment: Implementation

1) makes the original nsIAccessibleText functions defined in nsTextAccessible
as helper functions, and can accept a nsIDOMNode argument;
2) gets appropriate domNode from nsTextAccessible and
nsHTMLTextFieldAccessible;
3) calls helper function to implement nsIAccessibleText.

Seeking r=

Comment 2

[John Gaunt (redfive)]

Comment on attachment 91323 [details] [diff] [review]
Implementation

1) Figure out a way to not have to call GetTextNode in every function in
nsHTMLFormControlAccessible.cpp.  The best way I can think of would be to cache
the text node during creation of the object. This would save you all those
calls. 

2) Seem like there should be some way to work the inheritance so you don't have
to expose all the helper methods from nsTextAccessible. I realize
nsHTMLFormControl isn't actually text so it doesn't make sense to inherit from 
nsTextAccessible, but maybe something could be worked out. -- minor issue
really.

3) formatting -- some return statements are on the same line as an if(!object),
and some are after. Make them the same, at least on a per file basis. Since
there are already a bunch of instances of the return being on the next line, my
suggestion would be to adhere to that standard, rather than confuse the issue
by re-doing a bunch of other statements.

4) The comment by this method should either be before the signature, or inside
the block -- nsHTMLTextFieldAccessible::GetTextNode

fix/explain these and I'll r=

Comment 3

[Kyle Yuan]

Attachment: changed inheritance of nsTextAccessible and nsHTMLTextFieldAccessible

John, thanks your review.

Now, I changed the inheritance of nsTextAccessible and
nsHTMLTextFieldAccessible - let them both derive from a new class
nsAccessibleText that implemented nsIAccessibleText interface. That could avoid
to expose the helper function from nsTextAccessible and make the code smaller.
That's the answer of question 1) and 2).

I also fixed question 3) and 4)

Seeking r= again.

Comment 4

[John Gaunt (redfive)]

Comment on attachment 91431 [details] [diff] [review]
changed inheritance of nsTextAccessible and nsHTMLTextFieldAccessible

r=jgaunt. looks awesome, the change in inheritance worked out really well. :-)

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 91431 [details] [diff] [review]
changed inheritance of nsTextAccessible and nsHTMLTextFieldAccessible

sr=jst

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 91431 [details] [diff] [review]
changed inheritance of nsTextAccessible and nsHTMLTextFieldAccessible

a=asa (on behalf of drivers) for checkin to 1.1

Comment 7

[Kyle Yuan]

checked into trunk!