Open
Bug 86770
Opened 23 years ago
Updated 2 years ago
ALT,SPACEBAR is not activating control menu
Categories
(Core :: XUL, defect)
Tracking
()
NEW
People
(Reporter: tpowellmoz, Unassigned)
References
Details
(Keywords: access)
Attachments
(1 file, 2 obsolete files)
3.51 KB,
patch
|
emaijala+moz
:
review+
roc
:
superreview-
|
Details | Diff | Splinter Review |
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•23 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.
Reporter | ||
Comment 2•23 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.
Comment 4•23 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.
This should be pretty easy to add to nsMenuBarListener.cpp, at least I think it should...
Comment 6•22 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
Attachment #74057 -
Attachment is obsolete: true
Comment 10•22 years ago
|
||
Adding Aaron, since it's got the access keyword.
Comment 11•22 years ago
|
||
*** Bug 23445 has been marked as a duplicate of this bug. ***
Comment 12•22 years ago
|
||
Second call for reviews. It's quick and painless... anyone??
Comment 13•22 years ago
|
||
Adding Rod and Bill, known Windows dabblers, to check this out.
Comment 14•22 years ago
|
||
*** Bug 145562 has been marked as a duplicate of this bug. ***
Comment 15•22 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•22 years ago
|
||
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
Comment 17•22 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•22 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+
Comment 19•22 years ago
|
||
and r=me for that change
Comment 20•22 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.
Comment 21•22 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?
Comment 22•22 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•22 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•22 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.
Comment 25•22 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•22 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.
Comment 27•22 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?
Comment 28•22 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•22 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•21 years ago
|
||
*** Bug 197333 has been marked as a duplicate of this bug. ***
Attachment #92136 -
Flags: superreview?(sspitzer)
Comment 31•21 years ago
|
||
this bug also applies to mail.
Comment 32•21 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•21 years ago
|
||
what's the status here? Still looking for review from sspitzer. Can we check it in after that?
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.
Comment 35•21 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-
Comment 37•21 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•21 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: dean_tessman → nobody
Status: ASSIGNED → NEW
QA Contact: jrgmorrison
Comment 39•18 years ago
|
||
*** Bug 343869 has been marked as a duplicate of this bug. ***
Comment 40•18 years ago
|
||
sorry about that, I didn't realize that alt+space is different from alt, space.
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.widgets
Comment 42•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: dean_tessman → nobody
Updated•2 years ago
|
Severity: normal → S3
Comment 43•2 years ago
|
||
The severity field for this bug is relatively low, S3. However, the bug has 4 duplicates.
:enndeakin, could you consider increasing the bug severity?
For more information, please visit auto_nag documentation.
Flags: needinfo?(enndeakin)
Comment 44•2 years ago
|
||
Looking at other windows apps, Alt,Spacebar should not show the system menu, but it should activate the first menu, which we don't - space makes the focus leave the menubar (even if the menubar is showing).
So the bug here is that space should act like it's invoking the file menu.
Comment 45•2 years ago
|
||
The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.
Flags: needinfo?(enndeakin)
You need to log in
before you can comment on or make changes to this bug.
Description
•