Last Comment Bug 723833 - IAccessibleText::setCaretOffset on location or search bar causes focus to jump
: IAccessibleText::setCaretOffset on location or search bar causes focus to jump
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla13
Assigned To: alexander :surkov
:
:
Mentors:
Depends on:
Blocks: texta11y
  Show dependency treegraph
 
Reported: 2012-02-02 23:19 PST by James Teh [:Jamie]
Modified: 2012-02-09 08:58 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (9.29 KB, patch)
2012-02-03 02:52 PST, alexander :surkov
tbsaunde+mozbugs: review+
mzehe: review+
Details | Diff | Splinter Review

Description James Teh [:Jamie] 2012-02-02 23:19:41 PST
Str:
1. Press control+l to move to the location bar.
2. Type some text.
3. Get the accessible for the location bar.
4. Call IAccessibleText::setCaretOffset(0) on the location bar accessible.
Expected: The caret should move to the start of the text.
Actual: The focus moves, either to the top level frame or to the tab bar.
Note: Any caret offset (except the current caret offset) will cause this behaviour.

5. Press control+k to move to the search bar.
6. Type some text.
7. Get the accessible for the search bar.
8. Call IAccessibleText::setCaretOffset(0) on the search bar accessible.
Expected: The caret should move to the start of the text.
Actual: The focus moves, either to the top level frame or to the tab bar.
Note: Any caret offset (except the current caret offset) will cause this behaviour.

Tested in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120201 Firefox/13.0a1
Comment 1 Marco Zehe (:MarcoZ) 2012-02-03 01:22:06 PST
This is confirmed and can also be seen by using a braille display and trying to route the caret to any of the characters in the location bar or search field. Focus jumps to the tabs at the top of the window. This is a regression, since it used to work that one could route the caret via the routing buttons on a braille display.
Comment 2 James Teh [:Jamie] 2012-02-03 01:52:11 PST
Using a braille display is exactly how i found the bug. :)
Comment 3 alexander :surkov 2012-02-03 02:52:00 PST
Created attachment 594105 [details] [diff] [review]
patch

not sure I like solution but nothing better comes into my mind
Comment 4 Trevor Saunders (:tbsaunde) 2012-02-05 16:38:34 PST
(In reply to alexander :surkov from comment #3)
> Created attachment 594105 [details] [diff] [review]
> patch
> 
> not sure I like solution but nothing better comes into my mind

well, I'm not sure I like what we started with, particularly I dislike that we seem to just focus the next thing after we've moved the focus to do what we need to.

SOme questions I have are:
*what is supposed to happen if you set the  caret in something that isn't focused? should it become focused or should we fail?
* for that matter what about  the other callers? it seems more reasonable to want to copy / delete / paste text in something not focused than to set the caret.
* is it bad if we save the current focus move it to the thing we want to work on then restore it when we are done?  I'd think idealy selecting text and focus would be orthoginal, ubt maybe that's not possible and restoring the state after the task has been completed is the best we can do.
Comment 5 alexander :surkov 2012-02-05 20:41:40 PST
It seems caretOffset is expected to focus that's what we did basically. Due to some reason we don't focus when window is unfocused and probably we shouldn't change that.

It might be that we change focus for selection changes because of implementation details (we ask an editor to complete an action) but it sounds nobody cares.
Comment 6 Trevor Saunders (:tbsaunde) 2012-02-06 03:12:14 PST
Comment on attachment 594105 [details] [diff] [review]
patch

I think I'm fine with this change, but I don't have time to look at the tests in any detail until wensday so r=me on the C++ and r? marco on the tests.
Comment 7 Marco Zehe (:MarcoZ) 2012-02-06 03:23:50 PST
Comment on attachment 594105 [details] [diff] [review]
patch

r=me for the tests.
Comment 8 alexander :surkov 2012-02-07 20:29:12 PST
inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/0dbe46d2c255
Comment 9 Ed Morley [:emorley] 2012-02-08 08:59:46 PST
https://hg.mozilla.org/mozilla-central/rev/0dbe46d2c255
Comment 10 Marco Zehe (:MarcoZ) 2012-02-09 08:58:06 PST
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120209 Firefox/13.0a1

Note You need to log in before you can comment on or make changes to this bug.