319532 - [gnome]There is no feedback when use keyboard to navigate disabled menuitem in Mozilla menu

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Ginn Chen]

Steps:
1. Launch Firefox;
2. Press <Alt+E> to pop out Edit menu;
3. Use <Down> arrow key to select the menuitem;
4. When you navigate to a disable menuitem, there is no sign to show on this menuitem.
   For example, if there are six disabled options on this menu, use keyboard to navigate, there is no give feedback to user"Now we are in which option"

Expected result:
On gnome, disabled menuitem should be skipped when use keyboard to navigate.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This is a gnome theme issue, I guess? The disabled menu items are highlighted for me on Windows.

Comment 2

[Ginn Chen]

(In reply to comment #1)
> This is a gnome theme issue, I guess? The disabled menu items are highlighted
> for me on Windows.
> 

Right,
On gnome, disable menu items have no highlight background color, when you use mouse/keyboard to select it.

The point is:
On gnome, disabled menuitem should be skipped when use keyboard to navigate like other native gnome applications.
 

Comment 3

[[:Aleksej]]

It's not only in Firefox, but in Thunderbird 1.0.7 and 1.5rc2, too.

Comment 4

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

Attachment: patch

Comment 5

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

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

skip disabled menuitem

Comment 6

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

Attachment: patchv2

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> Created an attachment (id=211111) [edit]
> patchv2

As far as I know, Hyatt isn't reviewing Mozilla code, and hasn't for a while now. Unless you have some agreement with him about this, I think you're better off finding someone else to review.

Comment 8

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

Comment on attachment 211111 [details] [diff] [review]
patchv2

thanks, gavin

Comment 9

[Ginn Chen]

We need #ifdef for GTK and GTK2
We should not do so for Windows and other platforms

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 211111 [details] [diff] [review]
patchv2

I agree with Ginn, we should not take this change for all platforms.  This might be something that should go on nsILookAndFeel.

Comment 11

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

(In reply to comment #10)
> (From update of attachment 211111 [details] [diff] [review] [edit])
> I agree with Ginn, we should not take this change for all platforms.  This
> might be something that should go on nsILookAndFeel.
> 

Is there any standard that what's navigation rule and look&feel for disabled menuitem?

evolution skips disabled items while open office outlines them.

Comment 12

[Ginn Chen]

Fix should includes
UP/DOWN arrow keys
HOME/END keys
access keys

Comment 13

[Ginn Chen]

Attachment: a workaround

To draw menuitem as it does on Windows, although it's not as same as other gnome apps.

Comment 14

[Brian Ryner (not reading)]

Comment on attachment 213562 [details] [diff] [review]
a workaround

So this will just give us a hover/selected appearance if you arrow to a disabled item?  I guess that's better than arrowing there and getting no feedback.  I'd still really prefer a platform-specific behavior in the menu code to determine whether we allow disabled items to be selected at all.

Comment 15

[Ginn Chen]

Attachment: patch v3

Yesterday I thought this bug was too complex to be fixed easily.
Actually we can solve it by adding simple code.

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 213718 [details] [diff] [review]
patch v3

This should be ok but please add a comment that explains the reason for the #ifdef.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think I'd prefer to have new metric in nsILookAndFeel to control this.

Comment 18

[Ginn Chen]

Attachment: patch v4

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 215235 [details] [diff] [review]
patch v4

Good.

+  PRBool rv =

change 'rv' to 'result'. 'rv' usually is reserved for nsresult values.

Comment 20

[Ginn Chen]

Checking in layout/xul/base/src/nsMenuPopupFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v  <--  nsMenuPopupFrame.cpp
new revision: 1.280; previous revision: 1.279
done
Checking in widget/public/nsILookAndFeel.h;
/cvsroot/mozilla/widget/public/nsILookAndFeel.h,v  <--  nsILookAndFeel.h
new revision: 1.51; previous revision: 1.50
done
Checking in widget/src/gtk/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/gtk/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.65; previous revision: 1.64
done
Checking in widget/src/gtk2/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in widget/src/xpwidgets/nsXPLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/xpwidgets/nsXPLookAndFeel.cpp,v  <--  nsXPLookAndFeel.cpp
new revision: 1.48; previous revision: 1.47
done

Comment 21

[Ginn Chen]

Checking in layout/xul/base/src/nsMenuPopupFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v  <--  nsMenuPopupFrame.cpp
new revision: 1.259.2.4; previous revision: 1.259.2.3
done
Checking in widget/public/nsILookAndFeel.h;
/cvsroot/mozilla/widget/public/nsILookAndFeel.h,v  <--  nsILookAndFeel.h
new revision: 1.47.2.3; previous revision: 1.47.2.2
done
Checking in widget/src/gtk/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/gtk/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.63.20.1; previous revision: 1.63
done
Checking in widget/src/gtk2/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.23.8.1; previous revision: 1.23
done
Checking in widget/src/xpwidgets/nsXPLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/xpwidgets/nsXPLookAndFeel.cpp,v  <--  nsXPLookAndFeel.cpp
new revision: 1.44.2.1; previous revision: 1.44
done

Comment 22

[Adam Guthrie]

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