454377 - caret move event is fired for parent of html:textarea when textarea is tabed

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[Aaron Leventhal]

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

Don't we need it in
::SpellcheckSelectionChanged() as well?

Minor suggestion: sometimes it's hard to see the ! in |if(!blah()| (I missed it myself reading this the first time). That's why some people like to do:
if (! foo()) 
or
if (PR_FALSE == foo()) 
or
put in a comment to make sure readers don't miss that.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch2

with Aaron's comments

Comment 4

[Aaron Leventhal]

Comment on attachment 338846 [details] [diff] [review]
patch2

Thanks.

Comment 5

[Marco Zehe (:MarcoZ)]

Comment on attachment 338846 [details] [diff] [review]
patch2

> // Mapping needed state flags for easier handling.
> const state_focusable = 
>       Components.interfaces.nsIAccessibleStates.STATE_FOCUSABLE;
> const state_focused = 
>       Components.interfaces.nsIAccessibleStates.STATE_FOCUSED;
> const state_readonly = 
>       Components.interfaces.nsIAccessibleStates.STATE_READONLY;

Nit: Please use the nsIAccessibleState constant defined in common.js and get rid of the repetitive "Components.interfaces." part.

Comment 6

[alexander :surkov (:asurkov)]

checked in with Marco's comment

http://hg.mozilla.org/mozilla-central/rev/f7d15cc08499

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 338846 [details] [diff] [review]
patch2

patch makes more robust support of AT, it's safe because it fixes misreading of nsISelection interface.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 338846 [details] [diff] [review]
patch2

We have to bring in the 3.0.4 schedule, bumping to next release

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 338846 [details] [diff] [review]
patch2

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 10

[Marco Zehe (:MarcoZ)]

Alexander, can you take this one and the also approved bug 460417, please?

I tried applying the patch, and even after clearing the failed hunks, I still get complete failures in the test_events_caretmove.html file (it runs not ests, simply exits with some sort of invalid JS problem it does not specify).

This patch deals with stuff specific to 3.1 text attributes/spell checking support also, and I'm afraid to mess up things if I simply try to apply it.

Thanks!

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 338846 [details] [diff] [review]
patch2

Too late for 1.9.0.5.

Please don't request branch approval on patches that require further back-porting/merging. Request approval and what you intend to land.

Comment 12

[alexander :surkov (:asurkov)]

We don't need to port this bug to 1.9.0 because bug 460417 convers it. I'll put 1.9.0  patch for the bug 460417.