512561 - Can't set focus to document via accessibility APIs while Adobe Flash plugin has focus

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[James Teh [:Jamie]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090821 Minefield/3.7a1pre
Build Identifier: 

Once focus moves inside the Adobe Flash plugin, focus cannot be set to the parent Gecko document using accessibility APIs (for MSAA< accSelect with SELFLAG_TAKEFOCUS). This means that there is no way to programmatically return focus to the document. I suspect this applies to plugins other than Flash as well.

Reproducible: Always

Steps to Reproduce:
1. Visit the provided URL.
2. Move focus inside the Flash plugin; e.g. click the Play/Pause button.
3. Move up the ancestors of the focus accessible to the accessible for the containing document.
4. Try to set focus to this document accessible.
Actual Results:  
The focus does not move.

Expected Results:  
The document accessible should receive focus.

This bug is not present in Firefox 3.5.2.

Comment 1

[James Teh [:Jamie]]

Is this possibly related to bug 512058?

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Not sure, might also be related to bug 330412 and bug 78414.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

I wonder if we fail on !frame->IsFocusable() in nsAccessible::TakeFocus.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Ignore my comment #3.

Jamie are you doing an accSelect?

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Ugh ignore comment #4.  This is punishment for rick-rolling me :)

Comment 6

[James Teh [:Jamie]]

(In reply to comment #2)
> might also be related to bug 330412 and bug 78414.
It's certainly related in the sense that without those bugs fixed, this bug is even more important so at least ATs can get focus out of the plugin. :) However, those bugs also weren't fixed in Firefox 3.5.2, but at least set focus to the document used to work.

Comment 7

[Marco Zehe (:MarcoZ)]

I suspect this is a regression from the big focus refactor bug 178324 as well.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Josh, any ideas?

Comment 9

[Josh Aas]

Sorry, I don't have any ideas. Comment #7 seems like the best guess to me.

Comment 10

[Neil Deakin]

I suspect that nsDocAccessible::TakeFocus needs to be changed to ensure that the right window is focused just before (or after) it calls MoveFocus.

What should nsDocAccessible::TakeFocus be doing? Just focusing a document in an existing toplevel window? Or focusing the document and bringing its window to the front?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #10)
> I suspect that nsDocAccessible::TakeFocus needs to be changed to ensure that
> the right window is focused just before (or after) it calls MoveFocus.

Yes I think we need to do that.

> What should nsDocAccessible::TakeFocus be doing? Just focusing a document in an
> existing toplevel window? Or focusing the document and bringing its window to
> the front?

Given the typical use case, which is an AT user wanted to put focus somewhere for subsequent related activity, I think ensuring the window is front(most) is a Good Idea(tm).

Something like:
      return fm->MoveFocus(document->GetWindow(), nsnull,
                           nsIFocusManager::FLAG_RAISE |
                           nsIFocusManager::MOVEFOCUS_ROOT, 0,
                           getter_AddRefs(newFocus));

Neil, what does nsIFocusManager::MOVEFOCUS_ROOT do when focus is currently in a (flash) plugin?

Comment 12

[Neil Deakin]

> Something like:
>       return fm->MoveFocus(document->GetWindow(), nsnull,
>                            nsIFocusManager::FLAG_RAISE |
>                            nsIFocusManager::MOVEFOCUS_ROOT, 0,
>                            getter_AddRefs(newFocus));

Should work.

> Neil, what does nsIFocusManager::MOVEFOCUS_ROOT do when focus is currently in a
> (flash) plugin?

It should do the same thing, focus the root of the document.

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Jamie, if you want to see if raising the window helps: https://build.mozilla.org/tryserver-builds/[e-mail]-docroot_takefocus_raisewindow/docroot_takefocus_raisewindow-win32.zip

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

That was a red herring.

Debugging... looks like mDOMNode->GetOwnerDocument(getter_AddRefs(domDocument)) fails (domDocument is null).

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

Got it! Patch coming after lunch :)

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1

Get the nsIDOMDocument directly from the nsDocAccessible's dom node since this will succeed in more cases. GetOwnerDocument returns null if the caller node is in fact a document (this is probably new behaviour).

Jamie, nice catch.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

Forgot to mention I confirmed this fixes the bug as described.

Comment 18

[Marco Zehe (:MarcoZ)]

Comment on attachment 402856 [details] [diff] [review]
patch 1

Great! r=me

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

Push to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/88f7fdddc4ac

Comment 20

[David Bolter [:davidb] (NeedInfo me for attention)]

Let's bake it a bit before considering landing on 1.9.2

Comment 21

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090929 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Comment 22

[David Bolter [:davidb] (NeedInfo me for attention)]

Landed 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2a23df8cc57a

Comment 23

[Marco Zehe (:MarcoZ)]

Verified in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090930 Namoroka/3.6b1pre (.NET CLR 3.5.30729)