634240 - No caret move events are fired for XUL textbox accessible

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

regression from bug 615189

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

Marco: example in UI is text fields in Boorkmarks dialog in the case if you like to verify it.

Comment 3

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

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

r=me since this seems safe enough, but why doesn't the Accessible already have the correct node for this case? (nsIDOMXULTextBoxElement)

I see you made the change for all cases of FireAccessibleFocusEvent, not just xul, so it might be good to get some general QA as not sure how worried that should make us?

Tests look good.

>+++ b/accessible/src/base/nsRootAccessible.cpp
>@@ -512,16 +512,19 @@ nsRootAccessible::ProcessDOMEvent(nsIDOM

>+  nsIContent* origTargetContet = origTargetNode->IsElement() ?
>+    origTargetNode->AsElement() : nsnull;

Nit: forgot the 'n' in origTargetContent (4 places).

Comment 4

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

Looks like Alexander has builds here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-501bb8e36187/

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> Comment on attachment 512454 [details] [diff] [review]
> patch
> 
> r=me since this seems safe enough, but why doesn't the Accessible already have
> the correct node for this case? (nsIDOMXULTextBoxElement)

it has nsIDOMXULTextBoxElement but it's not correct node to register selection listener. XUL textbox is XUL wrapper on HTML input and all things are happen for HTML input while accessible is created for XUL textbox.

> 
> >+  nsIContent* origTargetContet = origTargetNode->IsElement() ?
> >+    origTargetNode->AsElement() : nsnull;
> 
> Nit: forgot the 'n' in origTargetContent (4 places).

thanks for the catch!

(In reply to comment #4)
> Looks like Alexander has builds here:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-501bb8e36187/

Yes :)

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #3)

> I see you made the change for all cases of FireAccessibleFocusEvent, not just
> xul, so it might be good to get some general QA as not sure how worried that
> should make us?

These changes don't make any sense since adding a selection controller for non text input controls don't make sense (it's handled by document). I just rollback part of bug 615189 for consistence.

Comment 7

[Marco Zehe (:MarcoZ)]

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

f=me. This bug definitely makes a difference and restores proper braille tracking with NVDA in the Bookmarks Library window and other places.

Comment 8

[alexander :surkov (:asurkov)]

landed on 2.0 (fx4beta12) - http://hg.mozilla.org/mozilla-central/rev/2e288f06c648

Comment 9

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre