ALT,SPACEBAR is not activating control menu

NEW
Assigned to

Status

()

Core
XUL
17 years ago
10 years ago

People

(Reporter: Tim Powell, Assigned: Dean Tessman)

Tracking

({access})

Trunk
x86
Windows 2000
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

17 years ago
ALT,spacebar does not activate the Windows control menu like ALT+spacebar does
(see bug 19328). The Windows control menu can also be triggered by clicking the
application icon at the left end of the title bar.

Steps to Reproduce:
1. Launch any Mozilla App.
2. Type ALT,SPACEBAR (press ALT, release ALT, hit spacebar)
(A variation on 2 that should also work is to press ALT, release ALT, arrow
right and left through the menus, and then press spacebar.)

Actual Result:
Focus goes to the menubar; other than that, nothing at all.

Expected Result:
Focus goes to the menubar, then the control menu activates so that a selection
can be made from it. This should behave similarly to how you can activate the
Edit menu by pressing Alt, Releasing Alt, and then pressing E.

Fails on Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.1+) Gecko/2001061

This almost certainly occurs on all MS-Windows OS.

The same sort of widget may be triggered the same way with some X Window
Managers - don't know.

This bug an almost exact duplicate of bug 23445. I don't think it makes sense to
reopen that one since it has been somewhat confusingly marked as a duplicate of
bug 19328 and has comments about multiple spacebar presses or using
ALT+Spacebar. Bug 19328 (ALT+Spacebar) is fixed. This one (or bug 23445 if you
prefer) is not.
(Reporter)

Comment 1

17 years ago
Adding appropriate keywords. This is 4xp (worked in Nav 4.x), access (multiple
key combos may be problematic for disabled users, this may be the only way to
move/resize windows if the user has no mouse, and this is a Windows OS
standard). Because of access concerns, proposing for 0.9.3.
Keywords: 4xp, access, mozilla0.9.3
(Reporter)

Comment 2

17 years ago
Drat! I meant to include this from bug 23445 (although it seems pretty obvious): 

Fixing this probably means making [spacebar] an accelerator key for the main
menubar, which then selects the same action that ALT+spacebar is triggering
directly.

i think this is a dupe
Status: NEW → ASSIGNED
Target Milestone: --- → Future

Comment 4

17 years ago
Just not note the heritage: bug 23445 was the original version of this bug 
report. It was duped to bug 19328 (which was for ALT+SPACE) as 'close enough'.
bug 19328 was fixed for ALT+SPACE, but ALT,SPACE was apparently dropped.

Not sure if there is an otherwise open bug for this, but I couldn't find one.
(Assignee)

Comment 5

17 years ago
This should be pretty easy to add to nsMenuBarListener.cpp, at least I think it 
should...

Comment 6

16 years ago
I think that this is an important bug for usability and accessibility.  I think 
that it should be moved from "Future" to Mozilla 1.01
(Assignee)

Comment 7

16 years ago
taking
Assignee: pinkerton → dean_tessman
Status: ASSIGNED → NEW
(Assignee)

Comment 8

16 years ago
Created attachment 74057 [details] [diff] [review]
This should do it.  Anyone care to review?
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: patch, review
(Assignee)

Comment 9

16 years ago
Created attachment 74059 [details] [diff] [review]
removed commented-out code.  reviews?
Attachment #74057 - Attachment is obsolete: true
(Assignee)

Comment 10

16 years ago
Adding Aaron, since it's got the access keyword.
(Assignee)

Comment 11

16 years ago
*** Bug 23445 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 12

16 years ago
Second call for reviews.  It's quick and painless... anyone??

Updated

16 years ago
Target Milestone: Future → ---
(Assignee)

Comment 13

16 years ago
Adding Rod and Bill, known Windows dabblers, to check this out.

Comment 14

16 years ago
*** Bug 145562 has been marked as a duplicate of this bug. ***

Comment 15

16 years ago
I tried it, but the patch doesn't apply cleanly. Dean, could you create a diff
that works for the current cvs?

Comment 16

16 years ago
Created attachment 92136 [details] [diff] [review]
Updated patch to apply cleanly to current tree

I updated the old patch to apply cleanly to current tree. It looks good to me,
but I had to make some changes to make it compile. Therefore it's maybe not
appropriate for me to r= it..
Attachment #74059 - Attachment is obsolete: true
(Assignee)

Comment 17

16 years ago
Was the only change from using aLetter to aKeyEvent->GetCharCode?  The change in
ShortcutNavigation from having aLetter as a parameter to aKeyEvent was in the
fix for bug 92491.

How about you review my code and I'll review your change?

Comment 18

16 years ago
Comment on attachment 92136 [details] [diff] [review]
Updated patch to apply cleanly to current tree

Yes, that's the only real change.

r=ere@atp.fi for everything but my lines of the patch.
Attachment #92136 - Flags: review+
(Assignee)

Comment 19

16 years ago
and r=me for that change

Comment 20

16 years ago
I want to see someone familiar with keys review this, such as akkana - not sure
it makes sense to hardcode this all into C++? I mean, aren't there similar
keystrokes to pull down the menu on the mac.. not sure about unix.
(Assignee)

Comment 21

16 years ago
Well, on Mac we use the native menu so we don't have to worry about handling 
keypresses there.  I can't speak for *nix, does it have keyboard access to the 
system menu?  Does it have a concept of a system menu?

Akkana, can you take a look at this re: Alec's comment?
(Assignee)

Comment 22

16 years ago
From IRC:

<bz> dean: well.. the unix thing is like this
<bz> dean: you hit a key
<bz> dean: the first thing that gets a crack at it is the X server itself
<bz> dean: it usually does nothing with it
<bz> dean: then the windowmanager gets a shot at it
<bz> dean: if _that_ does nothing with it, the app gets it
<dean> bz: so you already have keyboard access to the system menu?
<bz> dean: so the point is, the WM will do whatever it should do
<bz> dean: yes
<bz> dean: that's the upshot
<bz> dean: you would have to work very hard to prevent it

Alec, since this is only Windows-only, should we worry about the key being hard-
coded in the .cpp?  We could do something like the menu access key in 
nsMenuBarListener 
(http://lxr.mozilla.org/mozilla/source/layout/xul/base/src/nsMenuBarListener.cpp
#106), but that seems like overkill right now.

Comment 23

16 years ago
I'm not familiar with what the "control menu" is, but I have two questions after
looking at the patch:

1. If ALT+space was already working correctly, and it was just ALT,space that
needed work, why do we need this whole new #ifdef XP_WIN routine?  Can't
ALT,space do whatever ALT+space was already doing?

2. Are we absolutely sure that Windows is the only platform which will ever want
this functionality, and that no windows user would ever want to turn off the
functionality?  Generally we try to stay away from platform ifdefs in mozilla
code, and use dynamic libraries, LookAndFeel entries, prefs and other
runtime-changeable methods to control behavior.  What if OS2 or Be or some other
minor platform turned out to want the same behavior?  But if this is something
that can only have meaning on a Windows system, the ifdef may be warranted.  I
don't have a clear enough understanding of what this menu is to know.

Comment 24

16 years ago
the reason alt,space is special...
  alt-space is handled by the os
  alt,space should be handled by the os -- Except that we don't have an os
managed menubar, and we handle alt and then space on our own (or try to, this
bug is about the fact that we don't properly handle it) so the os doesn't.

So instead alt,space does nothing useful.  The only os's w/ system menus as such
are Windows, OS/2 PM and maybe QNX Photon (it's been a while, and i didn't
really fiddle with alt-space/alt,space while I was keying around Photon). I
think technically OS/2 is supposed to behave the same way as Windows (i can
check the next time I visit OS/2).

I can speak for BeOS, it doesn't have a system menu. BeOS behaves like macos
classic wrt widgets if you ignore the menubar's position.

If OS/2 and QNX Photon need this define, then we can make the ifdef generic or
we could just provide a stub that returns notimplemented/notavailable if you prefer.
(Assignee)

Comment 25

16 years ago
> 1. If ALT+space was already working correctly, and it was just ALT,space that
> needed work, why do we need this whole new #ifdef XP_WIN routine?  Can't
> ALT,space do whatever ALT+space was already doing?

When Alt+Space is pressed, we ignore it so it is handled by Windows so the
system menu pops up.  For Alt,Space though, pressing and releasign Alt activates
the menu bar and we handle all key strokes after that.  As timeless said, we
don't have an os-managed menu bar so we have to manually recreate this specific
functionality.

Comment 26

16 years ago
You can just think that pressing space while focus is on the menu bar (and
there's no menu open) should open system menu.
(Assignee)

Comment 27

16 years ago
akkana: Did you get your questions answered to your satisfaction?

mkaply: Does OS/2 need this as well?  Does it work on OS/2 by simply expanding
this #ifdefs to include OS/2?
(Assignee)

Comment 28

16 years ago
Akkana, Alec: we hard-code F10 in C++ in nsMenuBarListener.cpp.

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsMenuBarListener.cpp#248

In talking to users of other platforms this is only for Windows, but might be
for OS/2 because I haven't heard from anyone using that platform.

Comment 29

16 years ago
I believe this applies to OS/2 too, although I admit it's been a while since I
used OS/2.

Comment 30

15 years ago
*** Bug 197333 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Attachment #92136 - Flags: superreview?(sspitzer)

Comment 31

15 years ago
this bug also applies to mail.

Comment 32

15 years ago
I've got Moz 1.2.1. I haven't checked it recently, but it may assumingly be
possible that it might be with every window that in Windows has to be closed
with Alt+F4.

Comment 33

15 years ago
what's the status here? Still looking for review from sspitzer. Can we check it
in after that?

Updated

15 years ago
Attachment #92136 - Flags: superreview?(sspitzer) → superreview?(roc+moz)
+  PRUint32 charCode;
+  aKeyEvent->GetCharCode(&charCode);
+  if (charCode == NS_VK_SPACE)
+    ShowSystemMenu();
+  else

brendanize this by turning it into 

if (charCode == NS_VK_SPACE) {
  ShowSystemMenu();
  return NS_OK;
}

I think you should really move the Windows part of ShowSystemMenu to nsIWidget
and widget/src/windows/nsWindow.{h,cpp}. The rest of ShowSystemMenu in
nsMenuBarFrame should just call nsFrame::GetWindow(aPresContext, &window) and
then window->ShowSystemMenu(); that could even be inlined into the if() statement.
(Assignee)

Comment 35

15 years ago
I can move ShowSystemMenu into nsIWidget, sure, and just make it a no-op for
other platforms.  I'll still need an #ifdef XP_WIN in nsMenuBarFrame.h, if
you're OK with that, unless I add a second function to nsIWidget like
SupportsKeyboardSystemMenuAccess (exaggeration), but that seems like overkill to me.
> I'll still need an #ifdef XP_WIN in nsMenuBarFrame.h, if you're OK with that

Yeah, I'm OK with that. It's entire Windows-specific functions that bum me out.
Attachment #92136 - Flags: superreview?(roc+moz) → superreview-
(Assignee)

Comment 37

15 years ago
I finally got back to thinking about this.  roc, is it really necessary to move
ShowSystemMenu into nsIWidget if its only caller is wrapped in #ifdef XP_WIN?

Comment 38

15 years ago
ok, the system menu is in the menu loop for os/2 and needs to be triggered by
mozilla. so the api should probably be stubbed for non winish os's.

i'll check on photon some other time.
(Assignee)

Updated

14 years ago
Assignee: dean_tessman → nobody
Status: ASSIGNED → NEW
QA Contact: jrgmorrison

Updated

12 years ago
Assignee: nobody → dean_tessman

Comment 39

12 years ago
*** Bug 343869 has been marked as a duplicate of this bug. ***

Comment 40

12 years ago
sorry about that, I didn't realize that alt+space is different from alt, space.

Updated

10 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.