Closed Bug 319532 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: XUL, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

References

Details

(Keywords: fixed1.8.1)

Attachments

(3 files, 2 obsolete files)

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.
This is a gnome theme issue, I guess? The disabled menu items are highlighted for me on Windows.
(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.
 
Summary: This is no feedback when use keyboard to navigate disabled menuitem in Mozilla menu → [gnome]There is no feedback when use keyboard to navigate disabled menuitem in Mozilla menu
It's not only in Firefox, but in Thunderbird 1.0.7 and 1.5rc2, too.
Component: Menus → XML
Product: Firefox → Core
Version: unspecified → Trunk
Component: XML → XP Toolkit/Widgets: Menus
Attached patch patch (obsolete) — Splinter Review
Comment on attachment 211109 [details] [diff] [review]
patch

skip disabled menuitem
Attachment #211109 - Flags: superreview?(hyatt)
Attachment #211109 - Flags: review?(hyatt)
Attached patch patchv2 (obsolete) — Splinter Review
Attachment #211109 - Attachment is obsolete: true
Attachment #211111 - Flags: superreview?(hyatt)
Attachment #211111 - Flags: review?(hyatt)
Attachment #211109 - Flags: superreview?(hyatt)
Attachment #211109 - Flags: review?(hyatt)
(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 on attachment 211111 [details] [diff] [review]
patchv2

thanks, gavin
Attachment #211111 - Flags: superreview?(hyatt)
Attachment #211111 - Flags: superreview?(bryner)
Attachment #211111 - Flags: review?(hyatt)
Attachment #211111 - Flags: review?(bryner)
We need #ifdef for GTK and GTK2
We should not do so for Windows and other platforms
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.
Attachment #211111 - Flags: superreview?(bryner)
Attachment #211111 - Flags: superreview-
Attachment #211111 - Flags: review?(bryner)
Attachment #211111 - Flags: review-
(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.
Fix should includes
UP/DOWN arrow keys
HOME/END keys
access keys
Attached patch a workaroundSplinter Review
To draw menuitem as it does on Windows, although it's not as same as other gnome apps.
Attachment #211111 - Attachment is obsolete: true
Attachment #213562 - Flags: review?(bryner)
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.
Attachment #213562 - Flags: review?(bryner) → review+
Attached patch patch v3Splinter Review
Yesterday I thought this bug was too complex to be fixed easily.
Actually we can solve it by adding simple code.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #213718 - Flags: review?(bryner)
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.
Attachment #213718 - Flags: review?(bryner) → review+
Attachment #213718 - Flags: superreview?(roc)
I think I'd prefer to have new metric in nsILookAndFeel to control this.
Attached patch patch v4Splinter Review
Attachment #215235 - Flags: review?(roc)
Comment on attachment 215235 [details] [diff] [review]
patch v4

Good.

+  PRBool rv =

change 'rv' to 'result'. 'rv' usually is reserved for nsresult values.
Attachment #215235 - Flags: superreview+
Attachment #215235 - Flags: review?(roc)
Attachment #215235 - Flags: review+
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #213718 - Flags: superreview?(roc)
Attachment #215235 - Flags: approval-branch-1.8.1?(roc)
Attachment #215235 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
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
Keywords: fixed1.8.1
*** Bug 346437 has been marked as a duplicate of this bug. ***
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: menus → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.