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

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

11.49 KB, patch
Aaron Leventhal
: review+
MarcoZ
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Assignee)

Updated

10 years ago
Assignee: nobody → surkov.alexander
Summary: caret move event is fired for anonymous html:div when html:textarea is focused → caret move event is fired for parent of html:textarea when textarea is tabed
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

10 years ago
Created attachment 338843 [details] [diff] [review]
patch
Attachment #338843 - Flags: review?(aaronleventhal)
(Assignee)

Updated

10 years ago
Attachment #338843 - Flags: review?(marco.zehe)

Updated

10 years ago
Attachment #338843 - Flags: review?(aaronleventhal) → review+

Comment 2

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

Comment 3

10 years ago
Created attachment 338846 [details] [diff] [review]
patch2

with Aaron's comments
Attachment #338843 - Attachment is obsolete: true
Attachment #338846 - Flags: review?(aaronleventhal)
Attachment #338843 - Flags: review?(marco.zehe)

Comment 4

10 years ago
Comment on attachment 338846 [details] [diff] [review]
patch2

Thanks.
Attachment #338846 - Flags: review?(aaronleventhal) → review+
(Assignee)

Updated

10 years ago
Attachment #338846 - Flags: review?(marco.zehe)
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.
Attachment #338846 - Flags: review?(marco.zehe) → review+
(Assignee)

Comment 6

10 years ago
checked in with Marco's comment

http://hg.mozilla.org/mozilla-central/rev/f7d15cc08499
(Assignee)

Comment 7

10 years ago
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.
Attachment #338846 - Flags: approval1.9.0.3?
(Assignee)

Updated

10 years ago
Flags: in-testsuite+
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Attachment #338846 - Flags: approval1.9.0.4? → approval1.9.0.5?
Comment on attachment 338846 [details] [diff] [review]
patch2

We have to bring in the 3.0.4 schedule, bumping to next release
Comment on attachment 338846 [details] [diff] [review]
patch2

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #338846 - Flags: approval1.9.0.5? → approval1.9.0.5+
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 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.
Attachment #338846 - Flags: approval1.9.0.5+ → approval1.9.0.5-
(Assignee)

Comment 12

10 years ago
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.
Depends on: 460417
You need to log in before you can comment on or make changes to this bug.