363214 - nsIAccessible::TakeFocus() doesn't move focus properly

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Willie Walker]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061208 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061208 Minefield/3.0a1

If a text entry area inside an HTML form has focus, using the AT-SPI calls to setCaretOffset on some other component (e.g., a caret) should relieve that text area of its focus and position the caret where it was told to go.  Right now, an assistive technology can only programmatically move the caret outside a text area by forcing focus to some other focusable thing (e.g., another text area).

See also https://bugzilla.mozilla.org/show_bug.cgi?id=363198 for related discussion of setCaretOffset.  

Reproducible: Always

Comment 1

[Nian Liu(n/a in a long time)]

setcaretoffset should only change the caret offset, no focus change related.

setCaretOffset bug for both xul and html textbox was fixed in 359790

Comment 2

[Willie Walker]

(In reply to comment #1)
> setcaretoffset should only change the caret offset, no focus change related.
> 
> setCaretOffset bug for both xul and html textbox was fixed in 359790
> 

How does an assistive technology remove focus from a text box?  For example, suppose it wants to move the caret to a simple heading and prevent the text box from grabbing keyboard events.  We need to do this kind of thing in Orca because we are forced to write our own caret navigation due to shortcomings in the built in Firefox caret navigation. There seems to be something missing in the Firefox functionality to allow us to do this. 

Comment 3

[Nian Liu(n/a in a long time)]

(In reply to comment #2)
> (In reply to comment #1)
> > setcaretoffset should only change the caret offset, no focus change related.
> > 
> > setCaretOffset bug for both xul and html textbox was fixed in 359790
> > 
> 
> How does an assistive technology remove focus from a text box?  For example,
> suppose it wants to move the caret to a simple heading and prevent the text box
for a plain text, you can use top level accessible object to grab foucs, then set caret offset on the node.

set caret offset should only do the collapsed selection change in one node, not extra focus change work.

aaron and ginn, what's your opinion?

Comment 4

[Ginn Chen]

Will, so this bug is about there is no way to give focus back to HTML content from text entry, right?

I think it should be implemented by "grab focus" not "set caret pos".
Do you agree?


Yes, that's a bug.
"Grab focus" works for text entry, links, but no static text or html container.
 

Comment 5

[Nian Liu(n/a in a long time)]

> "Grab focus" works for text entry, links, but no static text or html container.

static text doesn't has a focus, so there is no "grab focus" for it. it's nothing wrong.

html container has a "grab focus" and works fine.

as i said in comment 3, i suggest use top level accessible object(html container) to do the focus change, then call plain text's setCaretOffset

Comment 6

[Ginn Chen]

ah,

grabFocus for "html container" or the parent of "document frame" work for me.

Will, is it solution OK?

Comment 7

[Willie Walker]

Orca has to move the caret around the content.  We want to allow people to be able to arrow in and out text areas as well as anything else.  When people arrow around, I have Orca do a grabFocus on the object and a setCaretOffset on the text if the object has accessible text.  

With the current model, I now need to have fragile logic:  if the caret moves to an object whose role is [some to be determined list of object roles that actually can get focus], then I can call grabFocus.  If the caret is not in this list, then I need to tell the document frame to grab focus and then set the caret position on the child/grandchild/... object where I want the caret to be.  What I'd really like to do is be able to say "put the caret here and tell what ever object it is to do the right thing with focus."

Comment 8

[Nian Liu(n/a in a long time)]

(In reply to comment #7)

> With the current model, I now need to have fragile logic:  if the caret moves
> to an object whose role is [some to be determined list of object roles that
> actually can get focus], then I can call grabFocus.  If the caret is not in
> this list, then I need to tell the document frame to grab focus and then set
> the caret position on the child/grandchild/... object where I want the caret to
> be.  What I'd really like to do is be able to say "put the caret here and tell
> what ever object it is to do the right thing with focus."
> 
in my understanding, the logic with current model should be something as below

1.first you know which node the caret is in and which node you want to set caret, right?

2.then if the node which will has the caret has a focusable attribute, call grabFocus and setCaretOffset.

3.if doesn't, look up a focusable object in parent chain, call the grabFocus on the focusable object and setCaretOffset on the node.

Comment 9

[Willie Walker]

> 2.then if the node which will has the caret has a focusable attribute, call
> grabFocus and setCaretOffset.
> 
> 3.if doesn't, look up a focusable object in parent chain, call the grabFocus on
> the focusable object and setCaretOffset on the node.

Sounds good to me.  I'll give it a try.  :-)  Thanks!

Comment 10

[Willie Walker]

Well...I tried.  Seems as though grabFocus on a document frame always wants to return False for me.  Should I be doing something different?

Comment 11

[Ginn Chen]

(In reply to comment #10)
> Well...I tried.  Seems as though grabFocus on a document frame always wants to
> return False for me.  Should I be doing something different?
> 

See my comment #6,

You can grabFocus on "html container" or the PARENT of "document frame" (should be ROLE_PANEL).
grabFocus on "document frame" does NOT work.

Comment 12

[Willie Walker]

> You can grabFocus on "html container" or the PARENT of "document frame" (should
> be ROLE_PANEL).
> grabFocus on "document frame" does NOT work.

The document frame exposes itself as FOCUSABLE and it also issues focus events.  Why doesn't it honor grabFocus?  Is there any chance of fixing this?  

Ideally, I'd just be able to tell the sub-component to grab focus.  I'm not sure why that isn't supported, and I'd like to hear a compelling reason for why it's not supported -- I might be missing something.  For example, is there a strong technical reason, such as "the guts of Gecko in that area are the meatballs and sauce for the spaghetti code that supports the unreliable and buggy caret navigation stuff"?

In any case, I'll see what I can do in Orca to accomodate this oddity.  Sure would be nice if I didn't have to hack so much, though.

By the way, the Gecko script for Orca is approaching 10% of the overall Orca code base.  A lot of that code is to support our own caret navigation model since Firefox's built-in one is so poor. :-(   Is there any hope that Firefox/Gecko will end up trying to do the right thing sometime in the future (I'll never give up hope)?

Comment 13

[Willie Walker]

I hacked around this by giving the panel focus (THANKS) and then dealing with having to ignore the resulting focus events from the panel.  However, there still seems to be a problem: I no longer get the blinking cursor in the static text area, even though I'm calling setCaretOffset on the text object.  It used to work.  How do I get the blinking cursor back?

Comment 14

[Nian Liu(n/a in a long time)]

document frame grabFocus seems to be bug. i'll take care of it.

for the caret blinking, i tested it with at-poke as below and it seems fine

1)set focus in some text field
2)call grabFocus on document frame'parent
3)call setCaretOffset on some static text
4)click window title bar of firefox and caret blinks at the specified offset

Comment 15

[Ginn Chen]

(In reply to comment #13)
> I hacked around this by giving the panel focus (THANKS) and then dealing with
> having to ignore the resulting focus events from the panel.  However, there
> still seems to be a problem: I no longer get the blinking cursor in the static
> text area, even though I'm calling setCaretOffset on the text object.  It used
> to work.  How do I get the blinking cursor back?
> 

Press F7 to turn caret browsing on.

Comment 16

[Nian Liu(n/a in a long time)]

Attachment: patch

anyother way to set document focus? this way doesn't seem so direct.

Comment 17

[Ginn Chen]

(In reply to comment #16)
> Created an attachment (id=249331) [edit]
> patch
> 
> anyother way to set document focus? this way doesn't seem so direct.
> 

should return error if GetRootFocusController fails

Comment 18

[Nian Liu(n/a in a long time)]

(
> should return error if GetRootFocusController fails
> 
reasonable. i'll modify it with aaron's comments

Comment 19

[Willie Walker]

(In reply to comment #15)
> Press F7 to turn caret browsing on.

Well...I guess asking me "is it plugged in?" is a fair question.  I have caret browsing on and I don't turn it off.

Comment 20

[Willie Walker]

Hey all - we seem to be getting to the point where this thread is going on way too long and we both think the other doesn't get the point.  I'm going to contact Ginn off line.  Ginn - I'm going to need to you try to work with this using Orca.  We can at least get on the same page that way.

Comment 21

[Ginn Chen]

(In reply to comment #20)
> Hey all - we seem to be getting to the point where this thread is going on way
> too long and we both think the other doesn't get the point.  I'm going to
> contact Ginn off line.  Ginn - I'm going to need to you try to work with this
> using Orca.  We can at least get on the same page that way.
> 

I've tried Orca HEAD.
And reproduced the caret invisible bug.
Thanks!

I'll look into it.
Keep in touch.

Comment 22

[Nian Liu(n/a in a long time)]

aaron, any chance to review the patch?

Comment 23

[Aaron Leventhal]

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

Does this work if another document previously had focus? For example, what if a document in a different tab had focus?

Comment 24

[Nian Liu(n/a in a long time)]

(In reply to comment #23)
> (From update of attachment 249331 [details] [diff] [review])
> Does this work if another document previously had focus? For example, what if a
> document in a different tab had focus?
> 

nope. do we need this be fixed?

Comment 25

[Ginn Chen]

(In reply to comment #24)
> (In reply to comment #23)
> > (From update of attachment 249331 [details] [diff] [review] [details])
> > Does this work if another document previously had focus? For example, what if a
> > document in a different tab had focus?
> > 
> 
> nope. do we need this be fixed?
> 

I don't think we want to show/hide tabs by grabFocus().
gedit doesn't do this, either.
Change the selection in "page tab list" will do that.

Comment 26

[Aaron Leventhal]

Okay, please check it in as it is. Just good to know.

Comment 27

[Nian Liu(n/a in a long time)]

Attachment: patch with ginn's comment

ginn, pls. help check it in

Comment 28

[Willie Walker]

I'm reopening this, though it may be more appropriate to open a separate bug.  The missing caret problem I describe in https://bugzilla.mozilla.org/show_bug.cgi?id=363214#c13 and Ginn confirmed in https://bugzilla.mozilla.org/show_bug.cgi?id=363214#c21 still seems to exist.

What I'm experiencing is this: if the caret is blinking when I'm in a paragraph of static text, Gecko does just about what I expect when I move the caret via calls to setCaretOffset.  This is because I'm not mucking around with focus at all since there is no need to change focus.  If I end up arrowing off the paragraph, the caret disappears.  This has two impacts:

1) A magnification user ends up losing sight of the caret, since the caret is now gone.
2) Hitting "tab" no longer takes me to the next thing after where I think the caret is. 

Just to confirm I'm doing the right thing, here's what I'm doing:

1) I first call grabFocus for the parent of the document frame as suggested in https://bugzilla.mozilla.org/show_bug.cgi?id=363214#c6.  I'm not sure this is an html container, nor do I see an html container in the ancestry of the object (see below).

2) I then call setCaretOffset on the object containing the static text (e.g., a different paragraph).

Note that the hierarchy I'm looking at is as so:

+-name='Minefield' role='application' state='' relations=''
  +-name='Bug 363214 – grabFocus to document frame doesn't remove focus from text entry areas (see comment #12) - Minefield' role='frame' state='ACTIVE BUSY ENABLED FOCUSABLE FOCUSED SENSITIVE SHOWING VISIBLE' relations='EMBEDS'
    +-name=None role='unknown' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
      +-name=None role='panel' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
        +-name=None role='scroll pane' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
          +-name='Bug 363214 – grabFocus to document frame doesn't remove focus from text entry areas (see comment #12)' role='panel' state='ENABLED FOCUSED SENSITIVE SHOWING VISIBLE' relations=''
            +-name='Bug 363214 – grabFocus to document frame doesn't remove focus from text entry areas (see comment #12)' role='document frame' state='ENABLED FOCUSABLE SENSITIVE SHOWING VISIBLE' relations=''
              +-name=None role='section' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
                +-name=None role='form' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
                  +-name=None role='section' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
                    +-name=None role='section' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''
                      +-name=None role='paragraph' state='ENABLED SENSITIVE SHOWING VISIBLE' relations=''

Should I be doing something else?

Comment 29

[Willie Walker]

Just to follow up some more - if I just call grabFocus on the first FOCUSABLE object above the paragraph, things seem to work as expected.  This object typically ends up being the document frame, so I'm guessing the advice from comment #11 must have changed?

Comment 30

[Nian Liu(n/a in a long time)]

willie

1.you don't need call document frame's parent, just call document frame. that what the patch solved.

2.pls. file another bug to trace the caret blinking issue. i'll take care of it.

Comment 31

[Nian Liu(n/a in a long time)]

willie, 

i can't reproduce the caret blinking bug. can you describe the reproducible steps in detail?

Comment 32

[Willie Walker]

Hi Nian:

Sorry - my reply in comment #29 was ambiguous.  I'm able to get the blinking caret now with this fix.  Thanks!  

I do see something else, however, where I cannot take focus off a link.  Orca is doing the following whenever it moves the caret:  first, it gives focus to the first FOCUSABLE object it finds in the ancestry (starting at the thing holding the caret and then looking upwards in the hierarchy) second, it calls setCaretOffset on the object where the caret is.  What I'm seeing is that if a link gets focus, it doesn't release focus until some other link or form widget gets focus.  

I'd like for a link to release focus when I programatically move the caret somewhere else.  This would emulate the default caret navigation behavior better.  Is the behavior I'm seeing a bug, or is there some other thing I need to do to make this happen?

Comment 33

[Nian Liu(n/a in a long time)]

willie, my test seems fine. can you post a test case with reproducing steps?

Comment 34

[Willie Walker]

Hi Nian: if you are able to run Orca, please add the following to your ~/.orca/orca-customizations.py file (creating a new file if necessary):
	
import orca.Gecko
orca.Gecko.controlCaretNavigation = True

Then, run Orca and Firefox nightly.  Make sure you have pressed F7 to enable caret navigation in Firefox.

1) Open a web page with links in it.  For example, the page for this bug.  

2) Click on the 'b' in the word 'grabFocus' at the top if this page.  

3) Arrow left to the link "Bug 363214".  You'll notice it gets focus (or is at least highlighted with a dotted rectangle).  

4) Arrow right so that the caret is no longer in "Bug 363214".  For example, arrow back to the 'b' in 'grabFocus'.  You'll notice that focus remains on the link.

Perhaps there's something I didn't understand in the descriptions in this bug and Orca should be doing something different.  What Orca is doing is described in comment #32.  Did I miss something?

Will

Comment 35

[Nian Liu(n/a in a long time)]

willie,

i sent you a seperate email to discuss the bug you described. once we have an agreement, we can file a new bug and fix it there.

however, since what you described is not related to this one. i'll close this bug.

Comment 36

[Willie Walker]

I'd like to reopen this bug.  There seems to be general problems with calling grab focus -- the calls do not seem to be honored.  Neo, do you have a working Orca environment so we can work together to resolve this?

Comment 37

[Nian Liu(n/a in a long time)]

will,

can you describe the reproduce steps?

Comment 38

[Willie Walker]

Hi Nian:

You need to run Orca, and you need to add the following to your ~/.orca/orca-customizations.py file (creating a new file if necessary).

import orca.Gecko
orca.Gecko.controlCaretNavigation = True

Then, run Orca and Firefox nightly.  Make sure you have pressed F7 to enable
caret navigation in Firefox.

1) Open the page for this bug.  

2) Click on the text area for "Additional Comments".  It should get focus.  

3) Press the up arrow.  Orca should speak the line above the text area, for example "Additional Comments".  When Orca does this, it has issued a setCaretOffset on the object with the text and a grabFocus on the appropriate focusable object holding the given text.  The grabFocus returns True.

4) You won't see the blinking caret move to the  "Additional Comments" area like it should.  Instead, the blinking caret stays in the text area.

5) Press the letter 'q'.  You'll see that it ends up entering text in the text area.

I would expect that the grabFocus on the appropriate object would:

1) Release focus on the text area, giving it to the appropriate object
2) Cause the caret to appear on the appropriate object

Comment 39

[Aaron Leventhal]

I have the non-obvious solution to this. Setting focus on the document should be easier in Gecko.

Comment 40

[Aaron Leventhal]

Attachment: Ugly, but it works. Should we put this into nsIDocument::SetFocus() so it can be reused? Also, if the top level window isn't active this can leave a focus outline ghost on an element

Comment 41

[Mats Palmgren (inactive)]

Comment on attachment 262144 [details] [diff] [review]
Ugly, but it works. Should we put this into nsIDocument::SetFocus() so it can be reused? Also, if the top level window isn't active this can leave a focus outline ghost on an element

I think this patch + orca is just wallpapering over a focus bug that really
is in the caret browsing code and should be fixed there.  I actually filed
a bug about that recently (bug 376369).  Basically, when caret browsing
in and out of text input fields the focus should move to, like we do for
links.

That said, the first part of the patch looks ok, but why do you move
the caret?  Removing everything from "Fix the tab order" forward seems
more correct to me.

Also, I got this assertion with this patch:
###!!! ASSERTION: No nsIAccessibleText for caret move event!: 'textAcc', file nsCaretAccessible.cpp, line 213

Comment 42

[Aaron Leventhal]

Mats, this bug doesn't really specifically relate to text fields. What I found is that focus wasn't correctly moving to the document via nsIAccessible::TakeFocus() when called on the document accessible.

What I did was copy how we move focus to a document in nsEventStateManager::ShiftFocusByDocument()

Moving the caret is necessary because the current position of the caret affects where the next tab/shift+tab moves to. This is true whether caret browsing is on or off.

Comment 43

[Aaron Leventhal]

Mats, I wasn't able to reproduce the assertion in comment #41

Regarding the MoveCaretToFocus() code, I guess we could remove that. It's mostly for those times where focus was on something like a link or non-editable control. The idea is that when you set focus to the document, the next tab press would move into the document, or shift+tab would move backwards out of the document.

However it sounds like the AT wants to explicitly control the caret, so it's probably better that we remove that. We can always add it back in if we need it.

Comment 44

[Aaron Leventhal]

Attachment: Remove caret movement code as Mats recommended (so current tab position will not be reset to root of document)

Comment 45

[Mats Palmgren (inactive)]

(In reply to comment #43)
> Mats, I wasn't able to reproduce the assertion in comment #41

Me neither right now - I'll file it separately if I see it again.

> Regarding the MoveCaretToFocus() code...
> The idea is that when you set focus to the document, the next tab
> press would move into the document, or shift+tab would move backwards out of
> the document.

A valid use case.  I was thinking that since we have separate methods
for setting focus and caret position in the non-a11y code I thought
it might be good idea to separate them here too.
IIRC, MoveCaretToFocus() collapses the selection.

> We can always add it back in if we need it.

Sure thing.

Comment 46

[Mats Palmgren (inactive)]

Comment on attachment 262722 [details] [diff] [review]
Remove caret movement code as Mats recommended (so current tab position will not be reset to root of document)

r=mats

Comment 47

[Joanmarie Diggs]

Aaron, what's the status of your patch?  grabFocus() *seems* to be more reliable in general, but not with form controls, so I'm wondering if I should wait or file new bugs. :-)

Thanks!!

Comment 48

[Aaron Leventhal]

I didn't know there was an issue with form controls -- only knew about the doc issues. Perhaps that got lost in the length of this bug. Will you open up a fresh bug for me?