Closed Bug 634240 Opened 9 years ago Closed 9 years ago

No caret move events are fired for XUL textbox accessible

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- final+

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: access, regression, Whiteboard: [hardblocker][has patch])

Attachments

(1 file)

regression from bug 615189
Attached patch patchSplinter Review
Attachment #512454 - Flags: review?(bolterbugz)
Marco: example in UI is text fields in Boorkmarks dialog in the case if you like to verify it.
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).
Attachment #512454 - Flags: review?(bolterbugz) → review+
blocking2.0: ? → final+
Whiteboard: [softblocker]
Whiteboard: [softblocker] → [hardblocker][has patch]
(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/surkov.alexander@gmail.com-501bb8e36187/

Yes :)
(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 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.
Attachment #512454 - Flags: feedback?(marco.zehe) → feedback+
landed on 2.0 (fx4beta12) - http://hg.mozilla.org/mozilla-central/rev/2e288f06c648
Flags: in-testsuite+
Target Milestone: --- → mozilla2.0b12
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.