341462 - Firefox fires "focus" event when mouse moves over menu while it is not focused

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Ginn Chen]

When Firefox is inactive (i.e. the window caption is grey), moving mouse over Firefox menu still emits "focus" event.

1) Open at-poke, click "Log events", mark "focus" checkbox.
2) Open firefox and gnome-terminal
3) Click gnome-terminal
4) Move mouse over firefox menu

Comment 1

[Ginn Chen]

This bug is not only on Linux but also on Window.
And even the firefox window is active, if the menuitem is not highlighted, we should not fire accessible focus event for "dommenuitemactive" event.
This is the behavior that common apps do on both Linux and Windows.

I think we should change these codes in nsXULMenuitemAccessible::GetState(PRUint32 *_retval)

  PRBool isFocused = PR_FALSE;
  element->HasAttribute(NS_LITERAL_STRING("_moz-menuactive"), &isFocused);
  if (isFocused)
    *_retval |= STATE_FOCUSED;

Comment 2

[Ginn Chen]

Attachment: a tricky patch for atk

GetFinalState, GetFinalRole is tricky for xulmenupopup and xulmenuitem
from theme's menu.css, we highlight menubar when it's open

TODO: Fix for msaa
      Fire focus event for dommenubaractive (we didn't deal with this event for atk yet)

Comment 3

[Ginn Chen]

Attachment: patch v2

This bug is more complicate than I thought.
This patch,
1) Solved FireCurrentFocusEvent issue
2) Made dommenuitemactive, dommenubaractive, dommenubarinactive work correctly on both Linux and Windows
3) Got STATE_COLLAPSED value for menuitem, also fixed stupid code there.
4) I have to add 1 line in nsMenuFrame::OpenMenu, because SelectMenu is called before OpenMenu.

Verified on Linux and Windows.

Comment 4

[Aaron Leventhal]

Why do we define it with an _ in front and then do this?
typedef struct _MaiUtilListenerInfo MaiUtilListenerInfo;

Comment 5

[Aaron Leventhal]

(In reply to comment #4)
> Why do we define it with an _ in front and then do this?
> typedef struct _MaiUtilListenerInfo MaiUtilListenerInfo;

I meant this comment for a different bug.

Comment 6

[Aaron Leventhal]

    // Currently we don't implement atkselection interface for menuxxx
    // So we can't emit selection_changed events for them

It's unclear if you're saying we should impl atkselection. If we are the comment should say XXX and we should file a bug for that.

Comment 7

[Aaron Leventhal]

QI's arereturn null for null. Instead of this:
+      if (domEventTarget) {
+        nsCOMPtr<nsIDOMNode> realFocusedNode(do_QueryInterface(domEventTarget));
+        mCaretAccessible->AttachNewSelectionListener(realFocusedNode);
+     

We do this:
+      nsCOMPtr<nsIDOMNode> realFocusedNode(do_QueryInterface(domEventTarget));
+      if (realFocusedNode) {
+        mCaretAccessible->AttachNewSelectionListener(realFocusedNode);
+

Comment 8

[Aaron Leventhal]

Instead of:
+   if (!isOpen)
+     *_retval |= STATE_COLLAPSED;
+   }

Do:
+   *_retval |= isOpen ? STATE_EXPANDED : STATE_COLLAPSED;

Comment 9

[Aaron Leventhal]

Comment on attachment 231073 [details] [diff] [review]
patch v2

Unfortunately, when I build with this patch, I no longer get focus events on objects with submenus (testing with MSAA on Windows).

For example, if you arrow down through the View menu you get focus events only for the leaf menu items (no submenu attached).

Comment 10

[Ginn Chen]

(In reply to comment #9)
> For example, if you arrow down through the View menu you get focus events only
> for the leaf menu items (no submenu attached).

Thanks for catching this.

Gnome application always opens submenu, when keyborard navigates to the menuitem. So they don't have this problem.
But Firefox only does it for the toplevel.

We need to add some code to deal with it.

Comment 11

[Ginn Chen]

Attachment: patch v3

Comment 12

[Ginn Chen]

(In reply to comment #6)
> It's unclear if you're saying we should impl atkselection. If we are the
> comment should say XXX and we should file a bug for that.
> 

I didn't see benefit of impl atkselction for menu.
We can just remove these lines.

Comment 13

[Aaron Leventhal]

Checked in for Ginn.

Comment 14

[Ginn Chen]

*** Bug 240736 has been marked as a duplicate of this bug. ***

Comment 15

[Aaron Leventhal]

This fix broke the focus events for menu items when just Alt or F10 is used to focus the menubar. I have filed bug 376640 for that.