Closed Bug 81727 Opened 23 years ago Closed 23 years ago

implement shift+F10 (Linux) / ctrl-space (Mac) to bring up context menu

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

x86
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
Future

People

(Reporter: bugzilla, Assigned: bryner)

References

Details

(Keywords: access, helpwanted)

Attachments

(3 files, 6 obsolete files)

summary sez it all. spun off from bug 74410. we need to have keycombo that'll bring up the context menu on linux [*nix] and mac platforms. shift+F10 has been suggested in http://www.mozilla.org/projects/ui/accessibility/mozkeyplan.html
i'd say this is a must-have for beta1 accessibility. :)
Severity: major → critical
Keywords: nsbeta1
aaron? is this really critical on mac and linux?
Keywords: access
Target Milestone: --- → mozilla1.0
iirc from my mtgs with aaronl and jeremy, this is one of the feature we want/need. :)
Target Milestone: mozilla1.0 → ---
From mpts comments in bug 36665, we need Control+Space on Mac as well.
I doubt if you can use Shift+F10 on Mac OS, either. On my Mac, Shift+F10 brings up the `Do you want to open the Keyboard control panel?' alert, so it would appear to be taken by the OS just like un-Shifted function keys are.
Linux should also have: F10-main menu <XK_Menu key> == context menu XK_Menu is mapped to <windows> menu key in XF86. Ctrl+Space is handled by the WM on X.
talked to aaronl, we can do this post-ship.
Keywords: nsbeta1nsbeta1-
Target Milestone: --- → mozilla0.9.3
easy -->dr
Assignee: pinkerton → dr
Severity: critical → major
Status: NEW → ASSIGNED
Priority: -- → P3
Post-ship, eh? ->1.0
Target Milestone: mozilla0.9.3 → mozilla1.0
[spam] dr@netscape.com's bugs subject to redistribution by chofmann. R!
Assignee: dr → chofmann
Status: ASSIGNED → NEW
Priority: P3 → --
Target Milestone: mozilla1.0 → ---
->aaronl, or trudelle, might you know who oughtta own this nowadays?
Assignee: chofmann → aaronl
pinkerton, jag?
cc'ing pink and jag to see if either of 'em would take this, this time around...
->pinkerton for later for mac work
Assignee: aaronl → pinkerton
Target Milestone: --- → mozilla1.0
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.9
Attached patch mac impl of shift-F10 (obsolete) (deleted) — Splinter Review
handle shift-F10 on mac to bring up a context menu. needing r/sr
cc'ing more folks for needed r/sr
Since when did Shift-F10 mean 'context menu' on Mac? F-keys are reserved for system use, and, anyway, how is the user to discover this magic combo?
for mac, mpt suggested control-space, i think that probably makes more sense. (As for XP bindings i think the solution is to allow people to select which platform's keybindings they want to use).
aaron, kathy, what Mac shortcut alternatives for shift+f10 would you suggest? would Control+Space be okay?
I defer to the Mac users out there ... Kathy, mpt, lordpixel, smfr, pink, saari, ....
I prefer control-space (though there may be issues in mac widget key code that prevent this from working (similar but different from 50255)). Pink--if you have a Mac widget key event problem, let me know; I'd be happy to fix it (I probably have a bug on my plate somewhere).
Summary: need to implement shift+F10 to bring up context menu on linux and mac → implement shift+F10 (Linux) / ctrl-space (Mac) to bring up context menu
Attached patch implement control-space on mac (obsolete) (deleted) — Splinter Review
ok. now using control-space instead of shift-f10. r/sr?
Attachment #66320 - Attachment is obsolete: true
Comment on attachment 66751 [details] [diff] [review] implement control-space on mac please fix this line: return ( inKeyEvent.charCode == kContextMenuKey && inKeyEvent.isControl ); Check for the other modifiers so that if a user presses more than control we won't hit this (maybe they were hoping to adjust keyboards with option-shift-space). r=brade with that change
Attachment #66751 - Flags: review+
Comment on attachment 66751 [details] [diff] [review] implement control-space on mac sr=sfraser with brade's rqeuirement to check other keys.
Attachment #66751 - Flags: superreview+
mac is done and landed, over to someone on the linux side.
Assignee: pinkerton → bryner
Status: ASSIGNED → NEW
Comment on attachment 66751 [details] [diff] [review] implement control-space on mac obsoleting last mac patch, it has been checked in.
Attachment #66751 - Attachment is obsolete: true
bryner, let me know if you don't have the cycles for this. akkana or jag, would you be able to do this if bryner can't? thx!
Keywords: nsbeta1
whee! verified control+space on mac 10.1.2 [2002.01.31] and 9.1 [2002.02.04] brings up the context menu. it appears in the upper-left of the content area, but hey, it works --i can arrow up/down to select the context menu item as well. thanks a bunch, pink!
Nominating for nsbeta1 triage.
nsbeta1- per Nav triage team, assuming this is only a problem now on Linux, where we don't have such strong accessibility requirements.
Keywords: nsbeta1nsbeta1-
Depends on: atfmeta
No longer depends on: atfmeta
Blocks: atfmeta
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.9 → Future
Keywords: helpwanted
mozilla works well with this patch in linux and solaris. Just a little difference with windows. In windows , the context menu is poped up at the location of the mouse, but in linux and solaris , the context menu is always poped up at the upper left corner of the focus widget. Should it be ? Need review?
Comment on attachment 73637 [details] [diff] [review] context menu key (shift-F10) implementation for linux/solaris r=bryner. ConvertKeyEventToContextMenuEvent is a bit contrived, but I can't think of anything better that isn't overkill.
Attachment #73637 - Flags: review+
Attached patch new patch of implement ation for context menu (obsolete) (deleted) — Splinter Review
oh, this new patch is neatly formatted by jst's tool "jst-review.pl" And , I change the OnKey function to : 1. find out whether the input key event is a context menu key, and if so, dispatch the NS_CONTEXTMENU_KEY event 2. if it is a normal key event, call the OnInput function to do the rest work. Maybe going to OnInput function will run some redundant codes, but if some one want to add/change some part of OnInput, he does not need to change the OnKey function, and it will less the possibility of creating new bugs. Hi, bryner, sorry to bother you again! would you review it again?
Attached patch new patch of implementation for context menu (obsolete) (deleted) — Splinter Review
oh, this new patch is neatly formatted by jst's tool "jst-review.pl" And , I change the OnKey function to : 1. find out whether the input key event is a context menu key, and if so, dispatch the NS_CONTEXTMENU_KEY event 2. if it is a normal key event, call the OnInput function to do the rest work. Maybe going to OnInput function will run some redundant codes, but if some one want to add/change some part of OnInput, he does not need to change the OnKey function, and it will less the possibility of creating new bugs. Hi, bryner, sorry to bother you again! would you review it again?
Attached patch new patch of implementation for context menu (obsolete) (deleted) — Splinter Review
oh, this new patch is neatly formatted by jst's tool "jst-review.pl" And , I change the OnKey function to : 1. find out whether the input key event is a context menu key, and if so, dispatch the NS_CONTEXTMENU_KEY event 2. if it is a normal key event, call the OnInput function to do the rest work. Maybe going to OnInput function will run some redundant codes, but if some one want to add/change some part of OnInput, he does not need to change the OnKey function, and it will less the possibility of creating new bugs. Hi, bryner, sorry to bother you again! would you review it again?
Attached patch new patch of implementation for context menu (obsolete) (deleted) — Splinter Review
oh, this new patch is neatly formatted by jst's tool "jst-review.pl" And , I change the OnKey function to : 1. find out whether the input key event is a context menu key, and if so, dispatch the NS_CONTEXTMENU_KEY event 2. if it is a normal key event, call the OnInput function to do the rest work. Maybe going to OnInput function will run some redundant codes, but if some one want to add/change some part of OnInput, he does not need to change the OnKey function, and it will less the possibility of creating new bugs. Hi, bryner, sorry to bother you again! would you review it again?
oh, this new patch is neatly formatted by jst's tool "jst-review.pl" And , I change the OnKey function to : 1. find out whether the input key event is a context menu key, and if so, dispatch the NS_CONTEXTMENU_KEY event 2. if it is a normal key event, call the OnInput function to do the rest work. Maybe going to OnInput function will run some redundant codes, but if some one want to add/change some part of OnInput, he does not need to change the OnKey function, and it will less the possibility of creating new bugs. Hi, bryner, sorry to bother you again! would you review it again?
I am so sorry about that. Our porxy always said that "time is out", so I attached it many times. Terribly!. The attachment 73666 [details] [diff] [review] 73667 73668 73669 should be obsolete and I have no such authority,so would someone please do it?
Attachment #73666 - Attachment is obsolete: true
Attachment #73667 - Attachment is obsolete: true
Attachment #73668 - Attachment is obsolete: true
Attachment #73669 - Attachment is obsolete: true
Comment on attachment 73670 [details] [diff] [review] new patch of implementation for context menu Please revise to follow style of files you are modifying: + nsWidget *widget = NULL; is "NULL" what is used elsewhere in this file or "nsnull"? + if (mEventCallback) { + // before we dispatch a key, check if it's the context menu key. + // If so, send a context menu event instead. + if ( IsContextMenuKey( aEvent ) ) { + nsMouseEvent contextMenuEvent; + ConvertKeyEventToContextMenuEvent( &aEvent, &contextMenuEvent); The above block doesn't appear to be consistent within itself for spaces near parentheses. remove extra spaces on this line: +// a context menu event. We want just about everything ( focused in the header, fix OnKey to be more like OnInput: + PRBool OnKey(nsKeyEvent &aEvent) ; Is the Mac bug being addressed in this bug or has it been moved to a separate bug or ?
Attachment #73670 - Flags: needs-work+
Gilbert, the context menu should open near the focused item, when it is brought up from a keyboard command. That is bug 81723 - another good one to fix.
NULL is wrong, even if it's used elsewhere in the file. (It can cause compiler warnings on some platforms.) Use 0 or nsnull.
Comments on the patch: This actually increases the redundancy since you end up rewriting the event to the focused widget twice for non-contextmenu keys. I would move that whole block of code inside the "if (IsContextMenuKey(aEvent))" block. Akkana, I think NULL will be correctly defined if you are including glib.h, but to be pedantically correct we should perhaps use NULL for GTK objects and nsnull for XPCOM objects; so that would mean nsnull in this case.
According to bryner's suggestion, the OnKey function should dispatch the non context menu key event directly instead of calling the OnInput function again. This will improve the performance comparing to the patch 73670. Also, nsnull is appropriatly used for nsWidget* and the extra spaces are deleted. bryner, please review it again? And the attachment 73637 [details] [diff] [review] and 73670 should be obsolete.
Comment on attachment 73852 [details] [diff] [review] amendatory patch of the last patch I probably would have used a #define instead of an enum for kContextMenuKey, but it's a style preference. r=bryner
Attachment #73852 - Flags: review+
Comment on attachment 73852 [details] [diff] [review] amendatory patch of the last patch sr=blizzard
Attachment #73852 - Flags: superreview+
Comment on attachment 73852 [details] [diff] [review] amendatory patch of the last patch a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73852 - Flags: approval+
checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
vrfy'd fixed on linux rh7.2 using 2002.03.26.08 comm bits.
Status: RESOLVED → VERIFIED
Verified also on build 2002040508, Mac OS 9.2. Filed bug 136634 as a follow-up.
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: