Last Comment Bug 751749 - cannot configure keyboard shortcuts to use Meta modifier instead of Alt
: cannot configure keyboard shortcuts to use Meta modifier instead of Alt
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86_64 Linux
: -- normal with 12 votes (vote)
: mozilla17
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
:
Mentors:
: 760291 (view as bug list)
Depends on: 819899 1023067
Blocks: 680830
  Show dependency treegraph
 
Reported: 2012-05-03 16:09 PDT by David A. Madore
Modified: 2014-06-09 22:18 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (11.36 KB, patch)
2012-06-12 22:34 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
Patch (10.92 KB, patch)
2012-06-12 22:38 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
Patch (11.45 KB, patch)
2012-06-13 22:58 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
part.1 Decide one modifier for a modifier flag (15.29 KB, patch)
2012-06-22 16:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
karlt: review-
Details | Diff | Review
part.2 Support Win key for a modifier of shortcut key and access key (35.31 KB, patch)
2012-06-22 17:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
bugs: review+
karlt: feedback+
Details | Diff | Review
part.3 Editor should handle Win key as a modifier key (23.83 KB, patch)
2012-06-22 17:02 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
part.1 Decide one modifier for a modifier flag (10.88 KB, patch)
2012-06-25 07:46 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
karlt: review+
Details | Diff | Review
part.4 Give higher priority to Meta than Super and Hyper due to better compatibility with Web applications (2.66 KB, patch)
2012-06-25 16:49 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
karlt: review+
Details | Diff | Review
part.1 Decide one modifier for a modifier flag (10.90 KB, patch)
2012-07-09 02:28 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review
art.2 Support Win key for a modifier of shortcut key and access key (35.26 KB, patch)
2012-07-09 02:29 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
enndeakin: review+
Details | Diff | Review
part.3 Editor should handle Win key as a modifier key (23.78 KB, patch)
2012-07-09 02:31 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
ehsan: review+
Details | Diff | Review
part.4 Give higher priority to Meta than Super and Hyper due to better compatibility with Web applications (2.67 KB, patch)
2012-07-09 02:31 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
no flags Details | Diff | Review

Description David A. Madore 2012-05-03 16:09:57 PDT
Short version: setting ui.key.menuAccessKey (and/or ui.key.accelKey) to 224 does not enable the "Meta" key for keyboard shortcuts, as expected.

More detailed explanation follows:

My keyboard layout has the "Meta" key bound to the X mod1 modifier.  Up to and including Firefox 12, navigation key shortcuts which are documented (e.g., on <URL: http://support.mozilla.org/en-US/kb/Keyboard%20shortcuts#os=linux&browser=fx13 >) as using the "Alt" modifier (Alt+Home, Alt+Left, Alt+Right and Alt+Digit) actually worked with "Meta" instead: this is fine for me because Alt+Home and other combinations are grabbed by my window manager.

Firefox 13 seems to have changed this: shortcuts which document the "Alt" modifier now actually demand "Alt" and not mod1.  (This is not a bug: this is the context explaining why the following bug is severe.)

In principle, this should be configurable via the ui.key.menuAccessKey: setting this key to 224 (=Meta; value taken from nsIDOMKeyEvent.idl) instead of the default 18(=Alt) should make "Meta" work for web page access keys and other "Alt"-documented shortcuts.  Unfortunately, this does not work (the shortcuts are not accessible at all).

Similarly, using 224 in ui.key.accelKey (default: 17=Control) does not make the "Meta" take over the function of the "Control" key.

In summary, "Meta" does not function for keyboard shortcuts (up to FF12 this was not a problem because it just looked for mod1, but now it insists on "Alt" and cannot be reconfigured to use "Meta").
Comment 1 Paul Murphy 2012-06-07 08:13:59 PDT
This bug also appears on the mozilla community support forums at https://support.mozilla.org/en-US/questions/928712
Comment 2 Paul Murphy 2012-06-07 08:39:17 PDT
This problem also affects Mac OSX Lion, https://bugzilla.mozilla.org/show_bug.cgi?id=760291
Comment 3 Paul Murphy 2012-06-07 21:57:30 PDT
(In reply to Paul Murphy from comment #2)
> This problem also affects Mac OSX Lion,
> https://bugzilla.mozilla.org/show_bug.cgi?id=760291

Correction:  OSX is not affected.  The OSX client in the previous link was connected to a Linux server running Firefox.
Comment 4 K Schoedel 2012-06-08 07:51:54 PDT
After a few minutes source browsing, I'm guessing this regression is due to the changes made for https://bugzilla.mozilla.org/show_bug.cgi?id=630813
Comment 5 K Schoedel 2012-06-11 07:46:19 PDT
Another possibility, 91 DOM_VK_WIN, also does nothing.
Comment 6 Karl Tomlinson (ni?:karlt) 2012-06-11 16:15:19 PDT
(In reply to David A. Madore from comment #0)
> My keyboard layout has the "Meta" key bound to the X mod1 modifier.  Up to
> and including Firefox 12, navigation key shortcuts which are documented
> (e.g., on <URL:
> http://support.mozilla.org/en-US/kb/
> Keyboard%20shortcuts#os=linux&browser=fx13 >) as using the "Alt" modifier
> (Alt+Home, Alt+Left, Alt+Right and Alt+Digit) actually worked with "Meta"
> instead: this is fine for me because Alt+Home and other combinations are
> grabbed by my window manager.

In trying to determine the appropriate action for you Meta key / mod1 modifier, it would be helpful to understand your keyboard layout.

Are you able to describe the hardware of your keyboard?
e.g. who made it?  model name?  perhaps you have a link to a photo?

Can you paste the output of "setxkbmap -print" and describe the version of xkeyboard-config (the package owning the files in /usr/share/X11/xkb), please?

Which window manager is this?
Does it describe its Alt+Home shortcut using "Alt" or something else?

How do other applications, particularly GTK applications describe the Alt and Meta keys and which one do they typically use? e.g. in the File menu of evince, on my system, "Properties" has an Alt+Return shortcut.  Is that labelled similarly on your system, and is it activated by your Alt or Meta key?
Comment 7 Karl Tomlinson (ni?:karlt) 2012-06-11 16:22:27 PDT
The output from xmodmap could also be helpful, please.
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-11 19:30:20 PDT
DOM-Meta key has been completely dropped because it's really different key from Command key on Mac.

What's DOM keycode generated on your physical Meta key now? Use following URL for confirming it.

> data:text/html,<ul id="list"></ul><script>window.addEventListener("keydown", function (e) { var log = "keycode=" + e.keyCode + " (0x" + e.keyCode.toString(16).toUpperCase() + ")"; var textNode = document.createTextNode(log); var li = document.createElement("li"); li.insertBefore(textNode, null); var ul = document.getElementById("list"); ul.insertBefore(li, ul.firstChild); setTimeout(function () { ul.removeChild(li); }, 5000); }, true);</script>

And we have changed the logic of computing DOM keycode of modifier keys in bug 447757. The new code uses unmodified keycode is used if a physical key has two or more modifiers. If you can, could you test on Aurora build too?
http://www.mozilla.org/en-US/firefox/channel/#aurora
Comment 9 David A. Madore 2012-06-12 05:57:18 PDT
Let me clarify about my keyboard layout.  Physically it's a standard PC keyboard, so there are three keys on the left of the space bar: from left to right, the standard control key (Xorg keycode 37), a key with a windows logo on it (Xorg keycode 133), and a key with "Alt" written on it (Xorg keycode 64).  This is quite standard.

What is less standard and perhaps confusing is that I configured the key with the "Alt" label to be a Meta key and the key with the windows logo to be "Alt": my .Xmodmap contains (among other things):

keycode 64 = Meta_L
keycode 133 = Alt_L
clear Mod1
add Mod1 = Meta_L
clear Mod2
add Mod2 = Alt_L

(The reason for this choice is that I'm accustomed to the Sun keyboard which has the Alt key to the left of the Meta key, see e.g. <URL: http://xahlee.org/emacs/i/kb/sun_keyboard_left.jpg >.  Also, it has long been traditional under PC's running Linux, especially among Emacs users, for the key labeled "Alt" to be actually Meta, until various user-friendly desktop environments started pushing for Alt instead.)

BUT this is rather irrelevant.  The fact that matters is that I have an Alt key _and_ a Meta key on my keyboard (and we can completely forget the fact that the Meta key happens to be labeled "Alt"), that Meta is bound to mod1 (this is also an old tradition) and Alt is bound to mod2.  And it so happens that I configured my window manager (which is fvwm) to grab all sorts of combinations with the Alt key as shortcuts (actually, fvwm sees it as mod2, not Alt, but that's also irrelevant): so I want my various programs to use Meta as primary modifier for shortcuts.

Until and including Firefox 12, it seems that Firefox just took mod1 as the Alt modifier, which is technically wrong but was fine with me (as in "two wrongs can make a right"), because mod1 is my Meta key and that's what I want to use for Firefox modifiers.  Now it seems to look for the Alt keysym.  I'm not blaming Firefox for any of this, of course!  I'm just explaining.

It's quite reasonable for Firefox to have made the decision that keyboard shortcuts would use the Alt modifier by default.  I'm not criticizing this.  The fact that my window manager grabs them for its own purposes is entirely my own doing.

However, what I'm saying is that I should be allowed to configure my own shortcuts with the Meta modifier instead, which is what I have and what I want to use.

The code snippet posted by Masayuki Nakano in comment #8 shows that my Meta key (Xorg keycode 64, keysym Meta_L) is seen by Firefox as keycode 224 (0xE0).  This is the key I want to use for my shorcuts in Firefox.  But if, for example, I use something like

<key id="goBackKb" keycode="VK_LEFT" modifiers="meta" command="Browser:Back" />

then it doesn't work (I can't activate this shortcut; whereas if I use "control" instead, it works).  So Meta doesn't work as a modifier, and that's what this bug is about.

Following your suggestion, I'll try with Firefox Aurora to see if it makes any difference.
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-12 06:17:52 PDT
(In reply to David A. Madore from comment #9)
> The code snippet posted by Masayuki Nakano in comment #8 shows that my Meta
> key (Xorg keycode 64, keysym Meta_L) is seen by Firefox as keycode 224
> (0xE0).  This is the key I want to use for my shorcuts in Firefox.  But if,
> for example, I use something like
> 
> <key id="goBackKb" keycode="VK_LEFT" modifiers="meta" command="Browser:Back"
> />
> 
> then it doesn't work (I can't activate this shortcut; whereas if I use
> "control" instead, it works).  So Meta doesn't work as a modifier, and
> that's what this bug is about.

Yeah, starting Fx13, event.metaKey returns false even if Meta key on Linux is pressed. The reason is, we guessed that if some web applications use the result for Mac users, it may make Linux users confused. The background is, DOM meta key is mapped to Command key on Mac. The Command key is common modifier key for shortcut. So, Meta key of Linux and Command key of Mac are really different. So, we worry the difference of the meanings. But if this bug is too serious for Linux users, it might be better to set meta key flag only when the computed keycode is Meta...
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-12 06:34:52 PDT
Ah, I forgot a fact. Starting Fx14, keyCode is never DOM_VK_META on Linux. Um, so, if we need to set metaKey true, we need to check whether the physical key has Meta modifier in some condition.
Comment 12 Adam Moore 2012-06-12 06:41:48 PDT
For me, it is a serious regression. It appears to make it impossible to map the mac keyboard correctly via preferences when launching on a Linux box.  See https://bugzilla.mozilla.org/show_bug.cgi?id=760291
Comment 13 K Schoedel 2012-06-12 06:57:12 PDT
In my case, I used both Linux and Mac, and I use the same physical key (USB Left GUI = 227, Right GUI = 231) for shortcuts on both systems, so that I am not constantly mis-typing commands. (KDE allows this, which is the main reason I use it.)

I use system's default configuration of modifiers. (Currently on Ubuntu 12.04)

$ xmodmap
xmodmap:  up to 4 keys per modifier, (keycodes in parentheses):

shift       Shift_L (0x32),  Shift_R (0x3e)
lock        Mode_switch (0x42)
control     Control_L (0x25),  Control_R (0x69)
mod1        Alt_L (0x40),  Meta_L (0xcd)
mod2        Num_Lock (0x4d)
mod3
mod4        Super_L (0x85),  Super_R (0x86),  Super_L (0xce),  Hyper_L (0xcf)
mod5        ISO_Level3_Shift (0x5c),  Mode_switch (0xcb)

$ setxkbmap -query
rules:      evdev
model:      pc104
layout:     us
variant:    mac
options:    lv3:ralt_switch_multikey,compose:paus,grp:caps_switch

xev reports my Command/Windows keys as:
  KeyPress event, serial 35, synthetic NO, window 0x5c00001,
    root 0x15d, subw 0x0, time 4222388063, (119,127), root:(699,1145),
    state 0x0, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

  KeyPress event, serial 35, synthetic NO, window 0x5c00001,
    root 0x15d, subw 0x0, time 4222389232, (119,127), root:(699,1145),
    state 0x0, keycode 134 (keysym 0xffec, Super_R), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

GTK calls the keys "Super" in shortcuts.

When I run the javascript test, I get

  keycode=0 (0x0)

for these keys. I don't think I really care what gets reported, as long as I can use it in ui.key.*.
Comment 14 David A. Madore 2012-06-12 08:08:09 PDT
Indeed, under FF15 Aurora, the Meta key now returns keycode 0 under Linux.

I have no opinion on what it should return, but I really think there should be some way to define shortcuts using the Meta key as a modifier.  I can see several approaches other than simply reverting to previous behavior:

* provide some way for users to configure what X11 keysym is mapped to what DOM key and/or modifier (so one could let Firefox pretend that the X11 Meta key is actually Alt, or force it to generate Meta even if that is not the default, or whatever),

and/or

* create new DOM key values and modifiers for the X11 modifiers (say DOM_VK_X11_META, DOM_KEY_X11_SUPER, DOM_KEY_X11_HYPER, and corresponding modifiers) and let the user sort it out.

I don't know which is preferable, but there should definitely be some way to handle the key.
Comment 15 Brian S 2012-06-12 09:31:09 PDT
For what it's worth, I'm using a Sun Type 7 USB keyboard attached via a KVM to a Sparc workstation, a Mac Mini, and a Dell running Linux.  The Meta key on the Sun keyboard is the little diamond key to the right of Alt on the left side, and to the left of the Compose key on the right side.  I'm primarily concerned with the left Meta, which I had reconfigured to use with Firefox.  When I use it with my Mac, that same key behaves as the Apple Command key, and I can use it as the modifier to open/close tabs, etc. in Firefox 13.  This is only a problem on the Linux machine.

Here's the output of the various requested commands on my Linux box:

sebby@mansquito:~% xmodmap
xmodmap:  up to 4 keys per modifier, (keycodes in parentheses):

shift       Shift_L (0x32),  Shift_R (0x3e)
lock        Caps_Lock (0x42)
control     Control_L (0x25),  Control_R (0x69)
mod1        Alt_L (0x40),  Alt_R (0x6c),  Meta_L (0xcd)
mod2        Num_Lock (0x4d)
mod3      
mod4        Super_L (0x85),  Super_R (0x86),  Super_L (0xce),  Hyper_L (0xcf)
mod5        ISO_Level3_Shift (0x5c),  Mode_switch (0xcb)

sebby@mansquito:~% setxkbmap -query
rules:      evdev
model:      pc105
layout:     us

Here's what I get in xev when I press and release the left meta key:

KeyPress event, serial 34, synthetic NO, window 0x3600001,
    root 0xbb, subw 0x0, time 505455605, (172,-10), root:(1043,495),
    state 0x10, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 34, synthetic NO, window 0x3600001,
    root 0xbb, subw 0x0, time 505455765, (172,-10), root:(1043,495),
    state 0x50, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

And right meta (which I'm not as concerned about using):

KeyPress event, serial 34, synthetic NO, window 0x3c00001,
    root 0xbb, subw 0x0, time 505809589, (164,-12), root:(1035,493),
    state 0x10, keycode 134 (keysym 0xffec, Super_R), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 34, synthetic NO, window 0x3c00001,
    root 0xbb, subw 0x0, time 505809829, (164,-12), root:(1035,493),
    state 0x50, keycode 134 (keysym 0xffec, Super_R), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False
Comment 16 Karl Tomlinson (ni?:karlt) 2012-06-12 15:28:57 PDT
GTK+ assumes that the Mod1 modifier is Alt and treats it that way:
http://git.gnome.org/browse/gtk+/tree/gtk/gtkaccelgroup.c?h=gtk-2-24#n1233
and will ignore Meta if it is on Mod1
http://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkkeys-x11.c?h=gtk-2-24#n1731

However, this can lead to inconsistencies when checking keysyms (instead of modifier bits) if the Alt_L/R keys are not bound to Mod1:
http://git.gnome.org/browse/gtk+/tree/gtk/gtkmain.c?h=gtk-2-24#n1656

so I'm not convinced that is a good model to follow.

(In reply to David A. Madore from comment #9)
> What is less standard and perhaps confusing is that I configured the key
> with the "Alt" label to be a Meta key and the key with the windows logo to
> be "Alt": my .Xmodmap contains (among other things):
> 
> keycode 64 = Meta_L
> keycode 133 = Alt_L
> clear Mod1
> add Mod1 = Meta_L
> clear Mod2
> add Mod2 = Alt_L
> 
> (The reason for this choice is that I'm accustomed to the Sun keyboard which
> has the Alt key to the left of the Meta key, see e.g. <URL:
> http://xahlee.org/emacs/i/kb/sun_keyboard_left.jpg >.  Also, it has long
> been traditional under PC's running Linux, especially among Emacs users, for
> the key labeled "Alt" to be actually Meta, until various user-friendly
> desktop environments started pushing for Alt instead.)
> 
> BUT this is rather irrelevant.  The fact that matters is that I have an Alt
> key _and_ a Meta key on my keyboard (and we can completely forget the fact
> that the Meta key happens to be labeled "Alt"), that Meta is bound to mod1
> (this is also an old tradition) and Alt is bound to mod2.  And it so happens
> that I configured my window manager (which is fvwm) to grab all sorts of
> combinations with the Alt key as shortcuts (actually, fvwm sees it as mod2,
> not Alt, but that's also irrelevant): so I want my various programs to use
> Meta as primary modifier for shortcuts.

Thanks for your explanation here.  While this is perhaps not *directly* related to the bug you report, I'd like to understand your situation better as it is a little different from the others commenting here, and I want to understand the importance of supporting a Meta modifier.

It sounds to me like you have changed the keysym name of your Alt key to Meta, and you want the Meta key/modifier to behave like it did when it was an Alt key.  Is that a fair summary?

I wonder whether the reason you did this is that you wanted the physical Alt key to behave like Meta in emacs?

Are you aware that if there is no separate Meta modifier, then emacs will treat the Alt modifier as Meta?

Usually xkb doesn't give PC keyboards a real Meta key but treats Mod1 as a "virtual" Meta (as well as Alt).

> However, what I'm saying is that I should be allowed to configure my own
> shortcuts with the Meta modifier instead, which is what I have and what I
> want to use.

That may be reasonable.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #10)
> But if this
> bug is too serious for Linux users, it might be better to set meta key flag
> only when the computed keycode is Meta...

If there is a Meta modifier distinct from Alt, then I expect it will be
harmless to set the meta modifier on the key evet.  I'm happy if you'd like to
do that, but I don't recall how easy that would be.  I expect there is a
different solution, at least for the others commenting here, and this Meta
modifier sounds like an unusual situation.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #11)
> Starting Fx14, keyCode is never DOM_VK_META on Linux.
> Um, so, if we need to set metaKey true, we need to check whether the
> physical key has Meta modifier in some condition.

Adding DOM_VK_META for a distinct Meta_L/R key sounds fine to me.
Comment 17 Karl Tomlinson (ni?:karlt) 2012-06-12 15:50:08 PDT
(In reply to K Schoedel from comment #5)
> Another possibility, 91 DOM_VK_WIN, also does nothing.

That should work from Firefox 15 as this changeset maps Super to VK_WIN:
http://hg.mozilla.org/mozilla-central/rev/c9ce0e49040a
That is currently on the Aurora channel.

That should be a solution for Brian also.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-12 22:34:59 PDT
Created attachment 632550 [details] [diff] [review]
Patch

How about this approach?

I think that it's no problem if Meta keys are independent keys and the modifier bits for Meta are not shared with other modifiers. So, this patch supports Meta key again.

However, some users may want to enable Meta keys even if the modifier bit is shared with other modifiers. E.g., XUL shortcut key hasn't support new modifiers. So, Meta might be shared with Super or Hyper, user may want to use the modifier as Meta. Therefore, this patch adds a new pref for such situation.
Comment 19 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-12 22:38:23 PDT
Created attachment 632551 [details] [diff] [review]
Patch

Oops, I forgot to do qrefresh, sorry.
Comment 20 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-12 22:44:21 PDT
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=752f19876bf8
Comment 21 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-13 00:24:48 PDT
test builds:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-752f19876bf8/
Comment 22 Adam Moore 2012-06-13 09:43:18 PDT
This build appears to address my issue (Bug 760291) along with providing an option to produce the behavior the original change meant to accomplish.  Nicely done.
Comment 23 Karl Tomlinson (ni?:karlt) 2012-06-13 16:14:07 PDT
Comment on attachment 632551 [details] [diff] [review]
Patch

>-    // If the keyval indicates it's a modifier key, we should use unshifted
>-    // key's modifier keyval.
>+    // If the keyval indicates it's a modifier key, we should compute the
>+    // DOM keycode from top priority modifier in all modifiers which are
>+    // activeted by the key.

Is this change a necessary part of this patch?

I would have thought that the unmodified keyval would be a good indication of
the top priority modifier, and that the keymap's idea of the most important is
would be better than making guesses in a predefined list.

>+pref("ui.key.disable_meta_if_flag_shared", true);

I'm not sure this is necessary, but I don't really object to you adding it.
Comment 24 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-13 19:10:47 PDT
(In reply to Karl Tomlinson (:karlt) from comment #23)
> Comment on attachment 632551 [details] [diff] [review]
> Patch
> 
> >-    // If the keyval indicates it's a modifier key, we should use unshifted
> >-    // key's modifier keyval.
> >+    // If the keyval indicates it's a modifier key, we should compute the
> >+    // DOM keycode from top priority modifier in all modifiers which are
> >+    // activeted by the key.
> 
> Is this change a necessary part of this patch?
> 
> I would have thought that the unmodified keyval would be a good indication of
> the top priority modifier, and that the keymap's idea of the most important
> is
> would be better than making guesses in a predefined list.

I think so. If we will support DOM_VK_META on GTK build again, I think that we should still hide the keycode as far as possible. For example, when a physical Alt key causes GDK_Meta_L for unshifted state and GDK_Alt_L for shifted state, then, shouldn't we generate DOM_VK_ALT? If the pref isn't changed, KeyboardEvent.metaKey returns false but KeyboardEvent.altKey returns true in this case.
Comment 25 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-13 19:26:06 PDT
> If the pref isn't changed, KeyboardEvent.metaKey returns false but KeyboardEvent.altKey returns true in this case.

I meant if the pref isn't change, KeyboardEvent.metaKey returns false but KeyboardEvent.altKey returns true in this case, but the keycode is DOM_VK_META.
Comment 26 Karl Tomlinson (ni?:karlt) 2012-06-13 21:16:52 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #24)
> If we will support DOM_VK_META on GTK build again, I think that
> we should still hide the keycode as far as possible. For example, when a
> physical Alt key causes GDK_Meta_L for unshifted state and GDK_Alt_L for
> shifted state, then, shouldn't we generate DOM_VK_ALT?

I don't think this is a situation that we are likely to encounter and
I don't want to add more logic to handle unlikely cases.

The Shift+Alt_L -> Meta_L situation is a special case that we should (and do)
handle, but all Meta-is-Alt layouts available from xkeyboard-config have Alt_L
in the unshifted level.

I expect that changing the reported key name is at least as likely to create
problems as solve any.
Comment 27 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-13 22:58:42 PDT
Created attachment 633034 [details] [diff] [review]
Patch

Okay, how about this?

This patch doesn't change the keycode computation in most cases. However, if the unmodified key causes DOM_VK_META, it searches preferred modifier. So, I think that this patch only prevents the case the keycode is DOM_VK_META but metaKey returns false.
Comment 28 Paul Murphy 2012-06-15 00:44:26 PDT
The test build in comment 21 did not respond to my Meta/Command key.  I set ui.key.accelkey = 224, and tried both values for the boolean ui.key.disable_meta_if_flag_shared.

Keyboard details follow:

Computer
=========
   Macbook-4.1, USA keyboard.

   Pertinent keys: Control_L, Alt_L, Command_L, Command_R, Alt_R

Operating System
================
   Ubuntu 10.04, Gnome 2 Desktop, GTK 2.30, with Compiz.

Gnome Keyboard Properties
=========================
   Keyboard: USA Macintosh

   Keyboard Model:  Generic 105 key int'l PC

   Control key is mapped to Control_l
   Alt     key is mapped to Super_l
   Command key is mapped to Meta_l and Alt_l  

Gconf: 
======
   Describes the Command key as both Super and Mod4

Masayuki Nakano's script from comment #8 (executed on Firefox 13)
=================================================================
   Control_L keycode  17 (0x11)
   Alt_L     keycode  18 (0x12)
   Command_L keycode   0 (0x0)

   Command_R keycode   0 (0x0)
   Alt_R     keycode   0 (0x0)

   Shift_L   keycode  16 (0x10)
   Shift_R   keycode  16 (0x10)

setxkbmap -print
=================
   xkb_keymap {
        xkb_keycodes  { include "evdev+aliases(qwerty)" };
        xkb_types     { include "complete"      };
        xkb_compat    { include "complete"      };
        xkb_symbols   { include "pc+us(mac)+inet(evdev)+level3(ralt_switch)+terminate(ctrl_alt_bksp)"   };
        xkb_geometry  { include "pc(pc105)"     };
   };

xmodmap:  up to 4 keys per modifier, (keycodes in parentheses):
========
   shift       Shift_L (0x32),  Shift_R (0x3e)
   lock        Caps_Lock (0x42)
   control     Control_L (0x25),  Control_R (0x69)
   mod1        Alt_L (0x40),  Meta_L (0xcd)
   mod2        Num_Lock (0x4d)
   mod3      
   mod4        Super_L (0x85),  Super_R (0x86),  Super_L (0xce),  Hyper_L (0xcf)
   mod5        ISO_Level3_Shift (0x5c),  Mode_switch (0xcb)

xev:  press the Command key
===========================
KeyPress event, serial 36, synthetic NO, window 0x6800001,
    root 0x105, subw 0x0, time 830214065, (60,70), root:(64,558),
    state 0x0, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 36, synthetic NO, window 0x6800001,
    root 0x105, subw 0x0, time 830214425, (60,70), root:(64,558),
    state 0x40, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

Evince
=======
   File menu "Properties" has Alt+Return shortcut, activated by the "Alt" key.
Comment 29 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-15 02:20:04 PDT
Paul:

I think that in your case, the key should work as Alt rather than Meta on Web applications. So, ui.key.accelkey = 18 should work as you expected. Could you check it? Anyway, I'll post new test builds with the new patch.
Comment 30 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-15 05:25:07 PDT
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-047ed15631ae/
Comment 31 K Schoedel 2012-06-15 07:14:40 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.
> com-047ed15631ae/

Trying this build, I see keycode=91 for the Command/Windows keys, but setting ui.key.accelkey=91 has no effect — shortcuts still use Control. (Did restart and run in safe mode of course.)
Comment 32 Paul Murphy 2012-06-15 07:44:39 PDT
(In reply to K Schoedel from comment #31)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> > https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.
> > com-047ed15631ae/
> 
> Trying this build, I see keycode=91 for the Command/Windows keys, but
> setting ui.key.accelkey=91 has no effect — shortcuts still use Control. (Did
> restart and run in safe mode of course.)

I also see keycode=91.  The Command/Windows key works for me with the following flags:

ui.key.accelkey=224
ui.key.disable_meta_if_flag_shared=false

I'm using a Gnome 2 desktop and also have to set the following in gnome-keyboard-properties:

Change Alt/Win key behaviour from "Default" to  "Meta is mapped to Win keys".

This does not change the Firefox keycode, but without it the Command key doesn't work in Firefox.
Comment 33 K Schoedel 2012-06-15 08:06:42 PDT
(In reply to Paul Murphy from comment #32)
> Change Alt/Win key behaviour from "Default" to  "Meta is mapped to Win keys".
> 
> This does not change the Firefox keycode, but without it the Command key
> doesn't work in Firefox.

That xkb flag changes the keysyms for keycodes 133/134 from Super_L/Super_R to Meta_L/Meta_R. This works for me in the FF16 build as it does for you, and doesn't seem to break any other programs I use, so I could live with that.
Comment 34 Paul Murphy 2012-06-15 08:13:52 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.
> com-047ed15631ae/

Command key: Firefox 16 keycode=91 (0x5B)

Flags set:
  ui.key.accelkey=224
  ui.key.disable_meta_if_flag_shared=false

To get the Command key to work, I also had to make a change in gnome-keyboard-properties:

Change Alt/Win key behaviour from "Default" to  "Meta is mapped to Win keys".

This does not change the Firefox keycode, but it does change the output of xev from Super_L to Meta_L.

Incidentally, after making the change to gnome-keyboard-properties, Firefox 13 and Firefox 16 give different keycodes!

Command key: Firefox 13 keycode=224 (0xE0)

(The Command key does not work in Firefox 13 with this keycode.) 

Thank you for solving this problem!  

Would it be possible to get the Firefox Command key to work with the Default setting in gnome-keyboard-properties (ie Super_L rather than Meta_L) ?  I think this would minimize the disruption to people running Gnome 2 on Apple keyboards.
Comment 35 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-17 19:23:07 PDT
Comment on attachment 633034 [details] [diff] [review]
Patch

Okay, I think that it's not enough to fix only in widget part. I think XUL's shortcut key handlers need to handle new modifiers especially nsIDOMKeyEvent::DOM_KEY_WIN.
Comment 36 Aaron Solochek 2012-06-21 11:24:33 PDT
I'm coming into this late, as I only recently allowed ubuntu to upgrade my thunderbird to 13.0.  I use the nostalgy add-on to assign shortcuts for moving messages around, and most of those rely on the meta (windows) key.  My modifiers are:

shift       Shift_L (0x32),  Shift_R (0x3e)
lock        Caps_Lock (0x42)
control     Control_L (0x25),  Control_R (0x69)
mod1        Alt_L (0x40),  Alt_R (0x6c),  Alt_L (0xcc)
mod2        Num_Lock (0x4d)
mod3      
mod4        Meta_L (0x85),  Meta_L (0xcd)
mod5        Scroll_Lock (0x4e)

Up to thunderbird 13 everything has worked perfectly for me.  I just wanted to chime in as someone else that relys on meta working.  I have no real choice but to stick to 12.0.1 until this is addressed in a new release.
Comment 37 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-22 16:56:34 PDT
Created attachment 635975 [details] [diff] [review]
part.1 Decide one modifier for a modifier flag

I think that a physical modifier key is related to only one modifier flag. And according to XUL key element, we shouldn't set two or modifiers to key events while only one physical modifier key is pressed. E.g., <key modifier="meta"> doesn't take key events whose modifier has both meta and another modifier. Therefore, I think that we should decide only one DOM modifier for the key.

This patch decides it with following rules:
1. Use a modifier which is computed from GDK keyval of the lowest level.
2. If two or more modifier keyvals are shared on a flag, use more important modifier.
Comment 38 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-22 17:00:40 PDT
Created attachment 635977 [details] [diff] [review]
part.2 Support Win key for a modifier of shortcut key and access key

For using Win keys as a modifier, we should support it on content accesskey, chrome acceskey, menu accesskey and acceleration key (shortcut key).

I want you to confirm the caption in menus of Win key on Linux. See toolkit/locales/en-US/chrome/global-platform/unix/platformKeys.properties
Comment 39 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-22 17:02:00 PDT
Created attachment 635979 [details] [diff] [review]
part.3 Editor should handle Win key as a modifier key

Editor should handle the Win key as a modifier like other modifiers.
Comment 40 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-22 17:11:31 PDT
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-06fddc9e1240/

test builds are here. I confirmed Meta and Win (Super/Hyper) are work as modifier key.

ui.key.accelKey and ui.key.menuAccessKey take one of following values:

17: Ctrl
18: Alt
224: Meta
91: Win (Super/Hyper)

ui.key.chromeAccess and ui.key.contentAccess take combination of following values:

1: Shift
2: Ctrl
4: Alt
8: Meta
16: Win (Super/Hyper)

Basically, unshifted GDK keyval decides DOM keycode and modifier. If a physical Alt key has GDK_Alt_L for unshifted position and GDK_Meta_L for shifted position, the test build decides it is DOM Alt. If a physical Win key has GDK_Meta_L for unshifted position and GDK_Super_L for shifted position, the test build decides it is DOM Meta key.
Comment 41 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-22 19:00:10 PDT
I think we need to do same things done in part.3 in all parts. But I think we should do it in next bug because it will need big patches but such bugs are less important than this bug.
Comment 42 Paul Murphy 2012-06-24 10:13:03 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #40)
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.
> com-06fddc9e1240/
> 
> test builds are here. I confirmed Meta and Win (Super/Hyper) are work as
> modifier key.
> 
> ui.key.accelKey and ui.key.menuAccessKey take one of following values:
> 
> 17: Ctrl
> 18: Alt
> 224: Meta
> 91: Win (Super/Hyper)
> 
> ui.key.chromeAccess and ui.key.contentAccess take combination of following
> values:
> 
> 1: Shift
> 2: Ctrl
> 4: Alt
> 8: Meta
> 16: Win (Super/Hyper)
> 
> Basically, unshifted GDK keyval decides DOM keycode and modifier. If a
> physical Alt key has GDK_Alt_L for unshifted position and GDK_Meta_L for
> shifted position, the test build decides it is DOM Alt. If a physical Win
> key has GDK_Meta_L for unshifted position and GDK_Super_L for shifted
> position, the test build decides it is DOM Meta key.

The Win/Command key now works with ui.key.accelkey = 224 & 19, but only if I first map the Win key to Meta in gnome-keyboard-properties.  If I use the default mapping of Win to Super, then Firefox detects the keycode 91, but the Win key does not work.
Comment 43 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-24 16:41:40 PDT
> If I use the default mapping of Win to Super, then Firefox detects the keycode
> 91, but the Win key does not work.

Did you reboot Firefox after changing the prefs? Excepting chromeAccess and contentAccess, other pref values are stored in static variables and they are never modified.
Comment 44 Paul Murphy 2012-06-24 20:23:36 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #43)
> Did you reboot Firefox after changing the prefs? Excepting chromeAccess and contentAccess, other pref values are stored in static variables and they are never modified.

I tested again, rebooting Firefox after each change of preferences, and got the same result.  I used Gnome 2's default keyboard mapping (Win -> Super), and tried ui.key.accelkey = 224 and 91.
Comment 45 Paul Murphy 2012-06-24 21:05:52 PDT
(In reply to Paul Murphy from comment #44)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #43)
> > Did you reboot Firefox after changing the prefs? Excepting chromeAccess and contentAccess, other pref values are stored in static variables and they are never modified.
> 
> I tested again, rebooting Firefox after each change of preferences, and got
> the same result.  I used Gnome 2's default keyboard mapping (Win -> Super),
> and tried ui.key.accelkey = 224 and 91.

CORRECTION:  The build in Comment #40 works on Linux with the default Gnome 2 keymap and ui.key.accelkey=91.  The Linux menus show the modifier as "Win",  eg "Win+T".

I retested in safe-mode and everything worked perfectly.

In Comment #42 and Comment #44 I was not using safe mode, and the Win key did not work.  Also the menus displayed "Meta" rather than "Win".  So one of my addons must be causing problems, most likely keyconfig.
Comment 46 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-24 21:22:42 PDT
Thank you for your testing.
Comment 47 Karl Tomlinson (ni?:karlt) 2012-06-24 23:49:22 PDT
Comment on attachment 635975 [details] [diff] [review]
part.1 Decide one modifier for a modifier flag

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #37)
> This patch decides it with following rules:
> 1. Use a modifier which is computed from GDK keyval of the lowest level.
> 2. If two or more modifier keyvals are shared on a flag, use more important
> modifier.

Sounds good.

But I don't think we need or want the mPreferredModifier changes for
ComputeDOMKeyCode.  In a sane setup, the results should be the same.

If they are different, I think it is more important to aim for accuracy in the
DOM keycode than to try to match the modifier.  If a keydown event is being
tested, then it is more likely being used to test which key is being pressed
than to test which modifier will become active.  If there is a page that
relies on the keycode/modifier relationship, then there will be another key
that the user can use to get the desired behavior.  If we were to change the
keycode, then there would be no other way for the user to generate the original keycode.  There would also be the (remote) possibility of DOM_KEY_LOCATION_*
inconsistencies.

>+        if (bit < 3 || bit > 7) {
>+            continue;
>+        }

bit is never > 7 here because the loop goes up to 8 * xmodmap->max_keypermod,
so there's no point checking that part.  Continuing for bit < 3 is fine.

>+    for (PRUint32 i = 0; i < COUNT_OF_MODIFIER_INDEX; i++) {
>+        Modifier modifier = NOT_MODIFIER;
>+        switch (i) {
>+            case INDEX_NUM_LOCK:
>+                modifier = NUM_LOCK;
>+                break;
>+            case INDEX_SCROLL_LOCK:
>+                modifier = SCROLL_LOCK;
>+                break;
>+            case INDEX_ALT:
>+                modifier = ALT;
>+                break;
>+            case INDEX_SUPER:
>+                modifier = SUPER;
>+                break;
>+            case INDEX_HYPER:
>+                modifier = HYPER;
>+                break;
>+            case INDEX_META:
>+                modifier = META;
>+                break;
>+            case INDEX_ALTGR:
>+                modifier = ALTGR;
>+                break;
>+        }
>+        for (PRUint32 j= 0; j < ArrayLength(mod); j++) {
>+            if (modifier == mod[j]) {
>+                mModifierMasks[i] |= 1 << (j + 3);
>+            }
>+        }
>+    }

Missing space after "j="

It took me a while to work out that the inner j loop would not be run for
shift/lock/control because there are no corresponding ModifierIndex values.
Please remove the modifier = NOT_MODIFIER initialization as that value will
never be used.  Perhaps add a default: NS_NOT_REACHED() case, but I'm not sure
that's necessary.

I'm wondering whether much of this file would be simpler (i.e. fewer switch
statements) if the bitmask Modifier enum were replaced with something like the
ordinal ModifierIndex enum.  That may be sensible only now that each state bit
will map to only one Modifier.  If adding (hard-coded) shift/lock/control
entries to mModifierMasks simplifies code, then that would be worth doing.
This change doesn't necessarily have to be made here, but is something to
think about.
Comment 48 Karl Tomlinson (ni?:karlt) 2012-06-24 23:58:19 PDT
Comment on attachment 635977 [details] [diff] [review]
part.2 Support Win key for a modifier of shortcut key and access key

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #38)
> I want you to confirm the caption in menus of Win key on Linux. See
> toolkit/locales/en-US/chrome/global-platform/unix/platformKeys.properties

Yes, "Win" sounds good to me.  KDE (or Qt) uses "Meta" (which would be problematic if there were Super and Meta) while GTK uses "Super", but xkb refers to the physical key as Win, so I think that is sensible.
Comment 49 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-25 07:46:21 PDT
Created attachment 636307 [details] [diff] [review]
part.1 Decide one modifier for a modifier flag

Um, it might be better to give higher priority to Meta than Super/Hyper.

Meta is usually mapped with Alt key, so, it's hidden in most cases for web applications. If I set "Meta to left Win key" in gnome keyboard layout option, the left Win key becomes Meta but right Win key is still Super, and they share same modifier Mod4. In this case, the older modifier "Meta" might be expected by users on some applications.
Comment 50 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-25 07:55:24 PDT
Comment on attachment 635977 [details] [diff] [review]
part.2 Support Win key for a modifier of shortcut key and access key

Currently, Super and Hyper keyvals on Linux (GTK) are mapped to DOM Win key. So, when either of them is active, KeyboardEvent.getModifierState("Win") returns TRUE. And also, the keyvals may be mapped to the Command keys on Mac keyboard such as comment 13.

So, we should support Win key is a modifier key for accesskeys and shortcut keys on Gecko for such users.

I think that I need to fix hundreds of parts which are checking if any modifiers are not activated (like part.3). However, I'd like to do it on next bug because it's less important than this bug and I need more days.
Comment 51 Karl Tomlinson (ni?:karlt) 2012-06-25 15:42:56 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #49)
> Um, it might be better to give higher priority to Meta than Super/Hyper.
> 
> Meta is usually mapped with Alt key, so, it's hidden in most cases for web
> applications. If I set "Meta to left Win key" in gnome keyboard layout
> option, the left Win key becomes Meta but right Win key is still Super, and
> they share same modifier Mod4. In this case, the older modifier "Meta" might
> be expected by users on some applications.

You might be right.
GDK seems to prefer Super
http://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkkeys-x11.c?h=gtk-2-24#n1692
but your reasoning makes sense.

The best info that I can find on this option is here:
http://bugzilla.xfree86.org/show_bug.cgi?id=1344#c3
Comment 52 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-25 16:49:21 PDT
Created attachment 636546 [details] [diff] [review]
part.4 Give higher priority to Meta than Super and Hyper due to better compatibility with Web applications

Thanks, then let's use this.
Comment 53 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-25 17:13:23 PDT
(In reply to Karl Tomlinson (:karlt) from comment #47)
> I'm wondering whether much of this file would be simpler (i.e. fewer switch
> statements) if the bitmask Modifier enum were replaced with something like
> the
> ordinal ModifierIndex enum.  That may be sensible only now that each state
> bit
> will map to only one Modifier.

Yeah, but one modifier can use two or modifiers. I think that it's not realistic situation because there are only 5 customizable modifier flags, so, some other modifiers are not mapped by the strange mapping. But our code should be able to handle such case. If we didn't handle such case, the code would look strange. For example, a later found modifier flag would or would not overwrite previous found one.

> If adding (hard-coded) shift/lock/control
> entries to mModifierMasks simplifies code, then that would be worth doing.

Yeah, maybe.
Comment 54 Neil Deakin 2012-06-26 10:26:45 PDT
Comment on attachment 635977 [details] [diff] [review]
part.2 Support Win key for a modifier of shortcut key and access key

It seems ok, but can you summarize what these patches are doing? Do they change any behaviour on other platforms besides gtk?
Comment 55 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-26 18:08:45 PDT
(In reply to Neil Deakin from comment #54)
> Comment on attachment 635977 [details] [diff] [review]
> part.2 Support Win key for a modifier of shortcut key and access key
> 
> It seems ok, but can you summarize what these patches are doing? Do they
> change any behaviour on other platforms besides gtk?

Sure. This patch just adds to support Win key as a new modifier key. The Win logo key is mapped to DOM Win key on Windows. And Super and Hyper keys are mapped to DOM Win key on GTK. No key is mapped to Win key on Mac.

This change is necessary for GTK users because some users may want to use the DOM Win key as a modifier, that is newly supported by D3E KeyboardEvent implementation. But if Windows users want to use Win key as a modifier key with the patched build, it's partially possible. But we don't kill the default behavior of Win key on widget for Windows, so, actually, it's difficult to use so. Therefore, basically, this change is useful only for GTK users.

Accessibly part: Adding Win key as a modifier for both accesskey and shortcut key.

nsContentUtils and toolkit part: Adding the caption for Win key. That is displayed in menus and so on.

nsEventManager part: Adding Win key support for both content and chrome accesskey (excepting accesskey in menu).

nsXBLPrototypeHandler part: Adding Win key support for shortcut keys. In this part, it's defined as "win" that the value for Win key of modifiers attribute of XUL <key>.

nsMenuBarListener part: Adding Win key support for menu accesskeys.

nsMenuFrame part: Adding Win key caption support for shortcut keys.

nsWindow for Win/GTK: They use the "key hell" path if Win modifier is activated too. This means we support all keyboard layout in the situation.
Comment 56 Neil Deakin 2012-06-27 07:45:51 PDT
Doesn't this patch make it so that shortcut keys can be made with the 'windows' key? I'm not sure we should be adding that as the windows key is meant for os use only.

What does pressing the windows key by default on linux do? Shouldn't it be considered to be the 'meta' key and be labelled as such (and not 'Win')?
Comment 57 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-27 08:22:33 PDT
(In reply to Neil Deakin from comment #56)
> Doesn't this patch make it so that shortcut keys can be made with the
> 'windows' key?

Yes.

> I'm not sure we should be adding that as the windows key is
> meant for os use only.

On Windows, I think so. But I don't think that we need to add #ifdef XP_UNIX for them.

> What does pressing the windows key by default on linux do? Shouldn't it be
> considered to be the 'meta' key and be labelled as such (and not 'Win')?

It depends on the system. Gnome3 is using Super key for activating "Activity". But I don't know if there are some system wide shortcut keys like Super+foo. I'm not sure about Hyper key too.
Comment 58 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-27 08:26:16 PDT
I meant that other guys in this bug have more knowledge about the Super and Hyper keys behavior than me.
Comment 59 Brian S 2012-06-27 08:29:05 PDT
Just out of curiosity, will the behavior of this patch be different for keyboards that have a real Meta key rather than a Windows key?  My Sun keyboard Meta key functions as the Windows key in Windows and the Command key in Mac, but I don't know if it actually sends a different keycode than the "real" version of those keys under the covers.
Comment 60 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-27 08:50:41 PDT
The environment described in comment 13 maps Mac keyboard's Command key as Super/Hyper key but they activate Mod4. On old our implementation, Mod4 always caused activating DOM Meta modifier. However, current our implementation detects the meaning of Mod1-5 with the applied native keycodes. This change helps D3E's KeyboardEvent.getModifierState() which distinguishes Meta and Win.

So, on such environment, we need to support DOM Win key (Super or Hyper on GTK) as a modifier because we cannot such key as Meta now. According to the fact that there is such environment, I think Super/Hyper should be able to be a modifier key on Linux desktop.
Comment 61 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-27 08:57:42 PDT
Brian:

Applications cannot know which physical key is what key. So, application only refer the keyval names like Meta_L and Super_L. So, it depends on your system settings. According to comment 15, your physical Meta keys are Win key for us. So, without part.2 patch, you cannot use the keys as a modifier key for shortcut keys and accesskeys.
Comment 62 K Schoedel 2012-06-27 09:01:01 PDT
(In reply to Brian S from comment #59)
> Just out of curiosity, will the behavior of this patch be different for
> keyboards that have a real Meta key rather than a Windows key?  My Sun
> keyboard Meta key functions as the Windows key in Windows and the Command
> key in Mac, but I don't know if it actually sends a different keycode than
> the "real" version of those keys under the covers.

These are all the same key as far as USB is concerned. It calls all such keys "GUI" and describes them as "Windowing environment key, examples are Microsoft Win key, Mac Apple key, Sun Meta key".

A normal USB keyboard (i.e. one without unusual firmware and matching driver on the host) can support only four pairs of modifiers — Shift, Control, Alt (Mac Option), and GUI (Windows Windows, Mac Command, Sun Meta).
Comment 63 Karl Tomlinson (ni?:karlt) 2012-06-27 17:43:55 PDT
(In reply to K Schoedel from comment #62)
> These are all the same key as far as USB is concerned. It calls all such
> keys "GUI" and describes them as "Windowing environment key, examples are
> Microsoft Win key, Mac Apple key, Sun Meta key".
> 
> A normal USB keyboard (i.e. one without unusual firmware and matching driver
> on the host) can support only four pairs of modifiers — Shift, Control, Alt
> (Mac Option), and GUI (Windows Windows, Mac Command, Sun Meta).

That's very interesting, thanks.  Are you able to link to any info on that, please?
Comment 64 Karl Tomlinson (ni?:karlt) 2012-06-27 18:09:05 PDT
(In reply to Neil Deakin from comment #56)
> Doesn't this patch make it so that shortcut keys can be made with the
> 'windows' key? I'm not sure we should be adding that as the windows key is
> meant for os use only.
> 
> What does pressing the windows key by default on linux do? Shouldn't it be
> considered to be the 'meta' key and be labelled as such (and not 'Win')?

By default X11 keymaps do not consider the windows key to be the Meta key.
By default the Alt key doubles as a Meta key.
But even when Meta is on the Alt key, Qt or KDE calls Mod4 "Meta".

The historic modifier story appears to be described reasonably well here:
http://lists.gnu.org/archive/html/texmacs-dev/2004-12/msg00003.html

Which parts of the system can use which modifiers is also less well defined.
Ctrl+Alt is sometimes used for system-wide shortcuts perhaps because at that time PC keys had no waving flag key.

There seems to be a move towards using Mod4 for window manager shortcuts, perhaps partially driven by NT OSs, and perhaps because applications have typically not used this modifier because there may be no such key mapped.

I think it would be sensible for applications to stay away from using Super for shortcuts where possible, and as a convention leave them for global shortcuts, but applications tend to run out of simple shortcuts.

This is complicated by the situation when Mod4 is configured as a Meta key.  In this situation, emacs keybindings that were on the Alt key move to the Meta key, so there are applications using Alt and applications using Meta.
(This also happens to be the situation when connecting remotely with a MacOS X11.)
It seems very sensible to allow configuring shortcuts for the Mod4 modifier when it is a Meta key.

When Mod4 is Super there is likely some debate.  We could try to force a convention where Super is reserved for global shortcuts, but there will be some people who want to configure shortcuts using that key.  Those people could change Mod4 to behave as Meta, but that may not be a preferred solution for everyone, because that will change the way emacs behaves.
Comment 65 K Schoedel 2012-06-27 20:27:33 PDT
> Are you able to link to any info on that, please?

Primarily, section 10 of the HID Usage Tables document, at <http://www.usb.org/developers/devclass_docs/Hut1_12v2.pdf>.
Comment 66 Karl Tomlinson (ni?:karlt) 2012-06-27 21:31:56 PDT
(In reply to Karl Tomlinson (:karlt) from comment #64)
> But even when Meta is on the Alt key, Qt or KDE calls Mod4 "Meta".

On MacOS, Qt's Meta is the key labelled control.
http://doc.qt.nokia.com/4.7-snapshot/mac-differences.html#special-keys

But DOM Level 3 events expect Meta to correspond to the key labelled command.
http://www.w3.org/TR/DOM-Level-3-Events/#key-Meta
Comment 67 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-06-27 23:50:18 PDT
(In reply to Karl Tomlinson (:karlt) from comment #66)
> (In reply to Karl Tomlinson (:karlt) from comment #64)
> > But even when Meta is on the Alt key, Qt or KDE calls Mod4 "Meta".
> 
> On MacOS, Qt's Meta is the key labelled control.
> http://doc.qt.nokia.com/4.7-snapshot/mac-differences.html#special-keys

I think that it makes simpler code of all Qt applications.

By mapping Control key to physical command key, it means that Qt applications need to handle only Control key for shortcut keys. It's smarter than DOM spec, but it's impossible to use such approach on Web due to backward compatibility.

DOM implementations honor the key label which is provided by keycode of native event. DOM Control key should be mapped to real Control key on each platform. Similarly, we're detecting the Mod1-5 meaning from the assigned keyvals on GTK. The merit of this approach is, DOM applications may not be confused by mismatching fired DOM key events and modifier state.
Comment 68 Olli Pettay [:smaug] 2012-07-03 16:09:37 PDT
Comment on attachment 635977 [details] [diff] [review]
part.2 Support Win key for a modifier of shortcut key and access key

Ok, DOM3 has "OS" modifier key.
Comment 69 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-09 02:28:23 PDT
Created attachment 640169 [details] [diff] [review]
part.1 Decide one modifier for a modifier flag

Updated for bug 769190.
Comment 70 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-09 02:29:16 PDT
Created attachment 640170 [details] [diff] [review]
art.2 Support Win key for a modifier of shortcut key and access key

enn: ping
Comment 71 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-09 02:31:13 PDT
Created attachment 640171 [details] [diff] [review]
part.3 Editor should handle Win key as a modifier key

Win key (OS key) should be changed as a modifier key for shortcut key or access key. So, editor should ignore the key events with Win key.
Comment 72 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-09 02:31:44 PDT
Created attachment 640172 [details] [diff] [review]
part.4 Give higher priority to Meta than Super and Hyper due to better compatibility with Web applications
Comment 73 Neil Deakin 2012-07-10 06:53:31 PDT
What I'm concerned about here is that, on Windows, one shouldn't be able to create a shortcut 'Windows+N', as is the case currently.

On Linux, one can map a unix-type key to the windows key, such that the generated key when the windows and N keys are pressed in actually 'Meta+N', for example. Should that be labelled in the UI as 'Win+N' or 'Meta+N'?

If one has configured the physical Alt key to be the virtual Control key, doesn't that get displayed in the UI as Control+N even though the user is actually pressing the key printed with Alt?
Comment 74 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-10 07:27:30 PDT
(In reply to Neil Deakin from comment #73)
> What I'm concerned about here is that, on Windows, one shouldn't be able to
> create a shortcut 'Windows+N', as is the case currently.

Yeah, but I think that most people won't customize the pref as so on Windows. And it's never changed accidentally. So, I don't think any additional limitation is necessary in the patches.

> On Linux, one can map a unix-type key to the windows key, such that the
> generated key when the windows and N keys are pressed in actually 'Meta+N',
> for example. Should that be labelled in the UI as 'Win+N' or 'Meta+N'?

If the native key indicates Unix's Meta key, that should be labelled "Meta+N" in UI. However, if the keys are mapped to Super or Hyper key, "Win+N" is better because they have mapped as so. ("Win" could be bad label in some cases, but even if we need to improve the label, we should do it later. It needs additional mechanism in nsContentUtils and/or widget. And it's not as urgent as this bug.)

> If one has configured the physical Alt key to be the virtual Control key,
> doesn't that get displayed in the UI as Control+N even though the user is
> actually pressing the key printed with Alt?

Yes, the label should be "Control+N" because we cannot know the actual key label of the physical key on all platforms. We can know only the virtual keycode which is converted from physical keycode with keyboard driver and/or something. Additionally, the user must have customized the settings themselves or known the mapping if they're using unusual settings. So, then, I think that if the label were "Alt+N", they would be confused conversely.
Comment 75 Neil Deakin 2012-07-10 10:35:10 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #74)
> Yeah, but I think that most people won't customize the pref as so on
> Windows. And it's never changed accidentally. So, I don't think any
> additional limitation is necessary in the patches.
> 

Doesn't this patch let an extension author write:

<key id="overrideSystemCommand" key="N" modifiers="os"/>

If so, then that's what I'm not sure we should be adding, and it doesn't seem like it should be added for this linux-specific bug.
Comment 76 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-10 16:40:08 PDT
(In reply to Neil Deakin from comment #75)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #74)
> > Yeah, but I think that most people won't customize the pref as so on
> > Windows. And it's never changed accidentally. So, I don't think any
> > additional limitation is necessary in the patches.
> > 
> 
> Doesn't this patch let an extension author write:
> 
> <key id="overrideSystemCommand" key="N" modifiers="os"/>
> 
> If so, then that's what I'm not sure we should be adding, and it doesn't
> seem like it should be added for this linux-specific bug.

Ah, did you mean I should drop "os" attribute value support from XUL <key>?
Comment 77 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-10 23:04:15 PDT
I think that it doesn't make sense to remove support "os" value from <key> element.

Meta modifier is never available on Windows, but XUL application developers can specify it on current implementation. If we should take care of the a11y risk in Gecko, we should also remove the "meta" but I don't think we should do it.

And we're providing "accel" for implementing shortcut keys. And "control" is really different key on Mac and the others. So, if addon developers wanted to use "os" and/or other specific modifier names, they must do it intentionally and carefully.

So, I don't think that we need to worry about such XUL applications.

If we don't support the "os" key, the demerit is, we cannot write automated tests for win key handling. The test in the patch helped me to write the patch. Therefore, I think that we should support "os" in XUL <key>.
Comment 78 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-12 19:33:57 PDT
enn: ping

This bug is very serious for some Linux users, so, I'd like to land these patches before next uplift in next week. Could you check it again?
Comment 79 C. Alex. North-Keys 2012-07-13 02:25:09 PDT
I have some suggestions - there's actually a much longer version of this with reasons and commentary on other posts (Nakano's, Karl's, and Deacon's) at http://www.talisman.org/~erlkonig/posts/2012-07-13-mozilla-x but I'll focus on just my summary suggestions here. 

Incidentally, I have keys for Alt, Meta, Super, and Hyper, and they all have specific uses.  Firefox's sudden failure to do Meta-<c> keys has been real killer.

It's easy to see how having both Alt and Meta act as equivalent command modifiers in Firefox worked well for Unix users, despite making Alt unavailable for Unicode mappings.  Alt == Meta is about a 90% solution, and implementation's straightforward.

My thoughts (although I must apologize for focusing mostly on X end-users rather than being more platform neutral or addressing extension authoring) is to stick with that in Unix, with some specifics:

- Users should be able to set the modifier used by Firefox outright.

  - Show only the modifiers available (in X, check xmodmap, show
    their current KeySym bindings, and ideally recheck them when
    appropriate by watcing for XMappingNotify).

- The "which-modifier" option *should* appear in the pretty-widgets UI.

- Let the user be explicit about modifiers.

  - When the user picks from the list { Option, Alt, Meta, mod1,
    mod2, ..}  save the exact choice made, but when using it, the
    Alt/Meta should be converted to a modN through the X modmap.
    This way, a user who chose Meta would be fine even if he moved
    it from mod1 to mod2, or conversely his choice of mod1 could be
    stable even if he didn't have a Meta or Alt.

  - There is a question about what to do if the explicitly chosen
    modifier isn't present.  What currently happens if both Alt and Meta
    are missing and mod1 is empty?  Does that guide what to do here? 

  - Saving picks breaks down a bit if the idea is to have preferences
    follow a user around between different OSes, unless the preferences
    themselves are also kept in per-OS-specific items.  If so, then
    it's fine.  Extension developers doing OS-agnostic mapping
    might explain the strange Super/Hyper = DOM Win mixing, in
    which case just letting users map DOM Win to either Super or
    Hyper internally to FF would be handy, but I may be totally
    missing the real point here.

- In X, be sure to focus on the key event state modifier mask.

  - Not some more creative-but-unstable approach of trying
    to track up/down state of keys based on KeySyms manually, or
    worse yet, KeyCodes.  Hopefully evdev will never even come up
    in this context.

- In X, store KeySyms or mod<N> rather then KeyCodes.

  - The user has enough to handle already getting them right in X
    without an application arguing that a keycode should map to
    some *other* character.

- In X, the Alt & Meta default makes the best sense, especially
    with the aforementioned ability of the user to override it.
Comment 80 David von Oheimb 2012-07-15 12:35:08 PDT
I'm pretty unhappy (to put it mildly) about this regression and went back to version 12 just because of this. BTW, strange that http://releases.mozilla.org/pub/mozilla.org/firefox/releases/12.0/ does not offer the official Linux release any more, and thus I had to resort to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/12.0b6-candidates/build1/linux-x86_64/en-US/
Comment 81 Neil Deakin 2012-07-16 07:08:37 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #77)
> If we don't support the "os" key, the demerit is, we cannot write automated
> tests for win key handling. The test in the patch helped me to write the
> patch. Therefore, I think that we should support "os" in XUL <key>.

But this is what I'm having trouble understanding. This is a Linux bug, but there is no 'OS' key on Linux, nor is there a 'Win' key, yet this patch appears to make it so that shortcuts appear labelled with 'Win' in the UI. Neither the bug's summary nor the original comment mention an 'OS' key nor a 'Win' key, it only talks about 'Alt' and 'Meta' which we already have modifiers for. I'm not clear why you're adding support for something else, and I don't see why we need to change anything on other platforms.
Comment 82 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-16 08:16:21 PDT
(In reply to Neil Deakin from comment #81)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #77)
> > If we don't support the "os" key, the demerit is, we cannot write automated
> > tests for win key handling. The test in the patch helped me to write the
> > patch. Therefore, I think that we should support "os" in XUL <key>.
> 
> But this is what I'm having trouble understanding. This is a Linux bug, but
> there is no 'OS' key on Linux, nor is there a 'Win' key, yet this patch
> appears to make it so that shortcuts appear labelled with 'Win' in the UI.
> Neither the bug's summary nor the original comment mention an 'OS' key nor a
> 'Win' key, it only talks about 'Alt' and 'Meta' which we already have
> modifiers for. I'm not clear why you're adding support for something else,
> and I don't see why we need to change anything on other platforms.

On Linux, there are only 5 undefined modifiers (not modifier keys) and 3 fixed modifiers such as Shift, CapsLock and Control. Our idea of deciding the meaning of the 5 undefined modifiers is, using the native keycode of physical keys which activate an undefined modifier flag. Typically, the unshifted native keycode is used for deciding the meaning.

If a user sets Meta key to Shift+Alt position (this is default settings of major distributions), the undefined modifier flag for the physical "Alt" key is decided as DOM Alt and the KeyboardEvent.metaKey never becomes true in such situation. And on such environment, typically, the physical "Win" logo keys are mapped to "Super" key. In this case, our idea thinks that an undefined modifier flag for the physical "Win" key is "Super" and "Super" is mapped to DOM "OS". So, if we didn't support "OS" as an accel modifier, when a user want to use the "Super" as accel key, he/she would meet same problem as this bug. For preventing such situation, I believe that we need to add support "OS" key as a modifier key which is available for shortcut keys.

Note that our old implementation (Fx11 and earlier) assumed that the undefined flag which is activated by physical "Win" key is DOM Meta. But such behavior doesn't make sense. If a key never causes DOM_VK_META key event, but such key activated KeyboardEvent.metaKey, this mismatch issue cannot explain how KeyboardEvent.getModifierState() works in D3E. When "Win" key causes DOM "OS" key event, that KeyboardEvent.getModifierState("OS") should return true but KeyboardEvent.getModifierState("Meta") should return false.

So, if we didn't support "OS" modifier as accel key modifier, some users who were using physical "Win" keys as "Meta" keys but actually mapped "Super" cannot use the "Win" key as an accel modifier key. And such settings are used in the default settings of major distributions.


Why we need to support "os" value in all platforms is, we have not made any differences of the values between any platforms. E.g., "meta" can be specified on Windows, but it never matches actually because any native key events on Windows don't set true for KeyboardEvent.metaKey. So, I don't think that we should insert |#if defined(XP_UNIX) && !defined(MAC_OS_X)| in a lot of places.
Comment 83 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-16 08:30:29 PDT
In other words, with major keyboard settings, the "Win" keys won't work as  DOM Meta if they are mapped to "Super" or "Hyper" keys. Instead, they will work as DOM OS due to the new modifier meaning detection code.

But on older Fx, they worked as DOM Meta even if the keycode isn't DOM_VK_META.

So, we really need to support "OS" as an accel modifier now.
Comment 84 Neil Deakin 2012-07-17 11:41:47 PDT
So all of this is just so that someone can, using preferences, configure the access key or accelerator key to 'super' or 'hyper'? Shouldn't the menu labels be 'Super' or 'Hyper' then instead of 'Win'?

Should we be adding Super and Hyper keys to the keyboard event interface? The events spec doesn't mention a windows (VK_DOM_WIN) key either, it just lists an 'OS' key.

It's very strange to me to map the Super and Hyper keys (which we don't actually need to support for most users at all) to an 'os' key with the VK_DOM_WIN key.

I think the test should be changing 'ui.key.menuAccessKey' and/or 'ui.key.accelKey' then performing the key tests, since that's what this bug is about.
Comment 85 Aaron Solochek 2012-07-17 12:09:17 PDT
This isn't simply about the menu access key or accelerator key.  This is also, at least for me, about the ability to assign hotkeys using the available keys/modifiers.  In my case Meta_L (0x85) with mod4 is what used to work but no longer works.
Comment 86 Karl Tomlinson (ni?:karlt) 2012-07-17 13:19:46 PDT
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html
currently says

"The 'Command' key modifier on Macintosh systems is represented using
[metaKey]."

but also has the almost contradictory specification referring to
'OS' as "The operating system key (e.g. the "Windows Logo" key)."

(In reply to Karl Tomlinson (:karlt) from comment #48)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #38)
> > I want you to confirm the caption in menus of Win key on Linux. See
> > toolkit/locales/en-US/chrome/global-platform/unix/platformKeys.properties
> 
> Yes, "Win" sounds good to me.  KDE (or Qt) uses "Meta" (which would be
> problematic if there were Super and Meta) while GTK uses "Super", but xkb
> refers to the physical key as Win, so I think that is sensible.

The level 3 events spec has changed, so I'm happy if this is renamed to "OS"
if that makes these changes more acceptable for review.

(In reply to Neil Deakin from comment #84)
> So all of this is just so that someone can, using preferences, configure the
> access key or accelerator key to 'super' or 'hyper'?

There are two different issues here.

The original reporter had configured a Meta modifier (in the OS and in the
app) and this is no longer supported in Gecko.

Other users had a modifier which was not Meta but used to "work" when the app
was configured to use Meta.

It does go beyond just preferences for builtin accelerators.
WebRT apps or web content may want to use additional modifiers.
I'm not familiar with the situation on "Windows" OS.  I'm a bit puzzled why the spec would include an "OS" modifier if that modifier is not available to apps.

> Shouldn't the menu labels be 'Super' or 'Hyper' then instead of 'Win'?

IMO no, because there is no Super or Hyper key on the keyboard and most users
do not know what Super or Hyper might be.  Given the DOM spec feels the need
to have an OS modifier, I expect it will be more useful to users to be able to
generate an OS modifier.

There are definitely different opinions here.  We could also argue that the
Mac Command modifier should be the OS modifier because common physical
keyboards do not distinguish between Command and Waving Flag (comment 62), but meta has been in the spec for some time.

If we can't promptly agree on the right way to handle the OS/Win/Super modifier,
one step that we could move in the right direction would be to support Meta when
there is a unique Meta modifier.
Comment 87 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-17 19:46:00 PDT
(In reply to Karl Tomlinson (:karlt) from comment #86)
> (In reply to Karl Tomlinson (:karlt) from comment #48)
> > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #38)
> > > I want you to confirm the caption in menus of Win key on Linux. See
> > > toolkit/locales/en-US/chrome/global-platform/unix/platformKeys.properties
> > 
> > Yes, "Win" sounds good to me.  KDE (or Qt) uses "Meta" (which would be
> > problematic if there were Super and Meta) while GTK uses "Super", but xkb
> > refers to the physical key as Win, so I think that is sensible.
> 
> The level 3 events spec has changed, so I'm happy if this is renamed to "OS"
> if that makes these changes more acceptable for review.

Do you want me to change the label to OS in the patch? If so, I'll change it.

> (In reply to Neil Deakin from comment #84)
> It does go beyond just preferences for builtin accelerators.
> WebRT apps or web content may want to use additional modifiers.
> I'm not familiar with the situation on "Windows" OS.  I'm a bit puzzled why
> the spec would include an "OS" modifier if that modifier is not available to
> apps.

I think that even if web developers don't want to *use* "OS" key as modifier, it's necessary on Windows. On Windows, most Win+foo don't do anything. However, some keys like Win+E, Win+F, Win+R, Win+D and Win+M are used for system wide shortcut keys. On our implementation, currently such operation don't cause the key events for the "foo" key. But other implementations might dispatch them (I'm not sure if it's possible technically). By this worry, web developers should be able to do nothing when they detect the "OS" modifier activated. For example, the part.3 is this situation, we shouldn't input text if Win key is pressed. And although I forgot the bug#, but there is a bug in tab group which needs to check the "OS" modifier state.

> There are definitely different opinions here.  We could also argue that the
> Mac Command modifier should be the OS modifier because common physical
> keyboards do not distinguish between Command and Waving Flag (comment 62),
> but meta has been in the spec for some time.
> 
> If we can't promptly agree on the right way to handle the OS/Win/Super
> modifier,
> one step that we could move in the right direction would be to support Meta
> when
> there is a unique Meta modifier.

I think that in my patches, Meta is enough respected. When "Meta" is mapped to unshifted position, "Meta" is more preferred than "Super" and "Hyper".

The "os" support is necessary for a modifier key which is mapped only "Super" and/or "Hyper" case. Then, users cannot use the key for neither accel modifier nor access modifier. And such keys are Windows logo key with default settings on Ubuntu and Fedora.
Comment 88 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-17 19:53:38 PDT
And the command keys on Mac keyboard on Ubuntu are mapped to Super and Hyper. So, they are mapped to DOM "OS". See comment 13, comment 17 and comment 31.
Comment 89 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-17 20:19:32 PDT
(In reply to Neil Deakin from comment #84)
> So all of this is just so that someone can, using preferences, configure the
> access key or accelerator key to 'super' or 'hyper'?

Yes. For most users, "Control" is enough. However, there are a lot of Linux environments. If they use non-PC keyboard, they need to customize the settings or prefs.

> Shouldn't the menu
> labels be 'Super' or 'Hyper' then instead of 'Win'?

I don't know if "Super" and "Hyper" labels are popular for modern Linux users.

> Should we be adding Super and Hyper keys to the keyboard event interface?
> The events spec doesn't mention a windows (VK_DOM_WIN) key either, it just
> lists an 'OS' key.

I don't think so currently. I'm planing that KeyboardEvent.key would return "OS" for "Super" and "Hyper" keys. If we add DOM Super and DOM Hyper, I think that most web developers don't handle them because most web applications are not tested on Linux (at least in Japan, I don't know in other countries). Therefore, I think that using "OS" key for "Super" and "Hyper" will provide better UX to our Linux users.

> It's very strange to me to map the Super and Hyper keys (which we don't
> actually need to support for most users at all) to an 'os' key with the
> VK_DOM_WIN key.

VK_DOM_WIN key has been named without D3E draft. It's my mistake, but the name isn't used by Web applications because all key name constants are not defined on other browsers. The key name constants are used only for internal usage and by addons. So, we don't need to worry about the mismatch of the names.

> I think the test should be changing 'ui.key.menuAccessKey' and/or
> 'ui.key.accelKey' then performing the key tests, since that's what this bug
> is about.

It doesn't make sense because these pref values require rebooting Fx :-( That's the reason why I don't add such tests in this bug. I think that we should do it later, but this bug should be fixed as soon as possible due to the importance.
Comment 90 Neil Deakin 2012-07-18 05:27:53 PDT
Comment on attachment 640170 [details] [diff] [review]
art.2 Support Win key for a modifier of shortcut key and access key

Ok, I think I understand what's going on here. It's quite confusing for a linux specific bug to have a patch about the windows key when the original comment doesn't mention it. Also, I didn't know that the meta key support had been changed/removed.

The 'Win' label still seems confusing, but I don't know what should be used instead. What happens if 'Super' is mapped to the physical 'Alt' key?

To be clear, I don't think we should support creating shortcuts for the windows key, nor for the hyper or super keys, except when the preferences are changed.
Comment 91 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-18 05:43:35 PDT
(In reply to Neil Deakin from comment #90)
> Ok, I think I understand what's going on here. It's quite confusing for a
> linux specific bug to have a patch about the windows key when the original
> comment doesn't mention it. Also, I didn't know that the meta key support
> had been changed/removed.

I'm sorry, I couldn't explain this enough for you. This series of bugs are too complicated.

> The 'Win' label still seems confusing, but I don't know what should be used
> instead. What happens if 'Super' is mapped to the physical 'Alt' key?

Then, the label will be "Win", but we cannot avoid such mismatch because we cannot know the physical key labels.

> To be clear, I don't think we should support creating shortcuts for the
> windows key, nor for the hyper or super keys, except when the preferences
> are changed.

I agree. I think that basically, UI designers (including addon developers) should use "accel" instead of individual modifier names. However, I also think that our application platform, XUL, should provide a way to specify all individual modifiers for some exceptional usage. When I document the change in MDN, I'll mention it.

Thank you, Enn.
Comment 92 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-18 05:45:44 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #87)
> (In reply to Karl Tomlinson (:karlt) from comment #86)
> > (In reply to Karl Tomlinson (:karlt) from comment #48)
> > > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #38)
> > > > I want you to confirm the caption in menus of Win key on Linux. See
> > > > toolkit/locales/en-US/chrome/global-platform/unix/platformKeys.properties
> > > 
> > > Yes, "Win" sounds good to me.  KDE (or Qt) uses "Meta" (which would be
> > > problematic if there were Super and Meta) while GTK uses "Super", but xkb
> > > refers to the physical key as Win, so I think that is sensible.
> > 
> > The level 3 events spec has changed, so I'm happy if this is renamed to "OS"
> > if that makes these changes more acceptable for review.
> 
> Do you want me to change the label to OS in the patch? If so, I'll change it.

I'll land the patches after karlt answers this. Karl, could you check it?
Comment 93 Karl Tomlinson (ni?:karlt) 2012-07-18 07:31:56 PDT
I don't have a preference on the Win/OS choice.

The advantage of "Win" is that it is consistent with much of the xkb configuration descriptions (which get used in desktop UI for modifier mapping configuration interfaces).

An advantages of "OS" is that it is consistent with the DOM APIs.  Another advantage is that it sounds less like an operating system that people are not using and may make more sense on keyboards that have a different symbol on the key (Mac keyboards, for example).
Comment 94 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-18 18:43:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6903f1a7df
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae94d3a4497e
https://hg.mozilla.org/integration/mozilla-inbound/rev/372c0dbbfb5b
https://hg.mozilla.org/integration/mozilla-inbound/rev/90f71c5e14d8

Okay, let's go with "Win" label for now. If there will be some suggestions, let's think again then.

And I'd like the victims of this bug to test the fix with Nightly builds (tomorrow's or later).
Comment 96 Francesco Lodolo [:flod] 2012-07-21 09:17:41 PDT
I tried to scan all the comments here but I didn't find anything useful. Is there a specific reason for having "win" on OS X and "Win" on Windows/Linux (different cases)?
Comment 97 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-21 16:50:09 PDT
(In reply to Francesco Lodolo [:flod] from comment #96)
> I tried to scan all the comments here but I didn't find anything useful. Is
> there a specific reason for having "win" on OS X and "Win" on Windows/Linux
> (different cases)?

See physical keyboards of Mac, they are using lower case for key names like "control". Anyway, Win key isn't available on MacOS X. So, the text will be never used except you set the accel key pref to Win key.
Comment 98 disposable 2012-07-24 15:02:39 PDT
Works here on Ubuntu 12.04 LTS.  Had to disable the 'Global Menu Bar integration' (3.3.1pre) extension to get it to work.
Comment 99 Adam Moore 2012-07-24 16:36:31 PDT
*** Bug 760291 has been marked as a duplicate of this bug. ***
Comment 100 Adam Moore 2012-07-24 16:37:21 PDT
The nightly is working for me.
Comment 101 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-07-24 17:05:37 PDT
Thank you for your confirmation!

(In reply to andrew.t.baldwin from comment #98)
> Works here on Ubuntu 12.04 LTS.  Had to disable the 'Global Menu Bar
> integration' (3.3.1pre) extension to get it to work.

Did Fx11 or earlier work with that?
Comment 102 Aaron Solochek 2012-07-24 18:13:48 PDT
I downloaded the nightly 2012-07-23-03-06-06-comm-central and tested it with the addon that this bug broke, and it is still not functioning correctly.  The addon is nostalgy ( http://alain.frisch.fr/index.html ), and I tried whatever the current version on mozilla.org is, and also the development release from that link, and both had the same result.

When I attempt to set a shortcut using meta, by grabbing a key combo, typing meta-f results in "meta META."  Apparently my meta (mod4, Meta_L (0x85)) is being detected as both the modifier and the regular key, because the "meta META" is being captured as soon as I press meta, before I even have a chance to press the "f."
Comment 103 disposable 2012-07-24 19:10:24 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #101)
> Thank you for your confirmation!
> 
> (In reply to andrew.t.baldwin from comment #98)
> > Works here on Ubuntu 12.04 LTS.  Had to disable the 'Global Menu Bar
> > integration' (3.3.1pre) extension to get it to work.
> 
> Did Fx11 or earlier work with that?

Version 3.2 of the extension (the version from the same time period) works with Firefox 11.
Comment 104 disposable 2012-07-25 18:00:31 PDT
Either reboot(In reply to andrew.t.baldwin from comment #103)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #101)
> > Thank you for your confirmation!
> > 
> > (In reply to andrew.t.baldwin from comment #98)
> > > Works here on Ubuntu 12.04 LTS.  Had to disable the 'Global Menu Bar
> > > integration' (3.3.1pre) extension to get it to work.
> > 
> > Did Fx11 or earlier work with that?
> 
> Version 3.2 of the extension (the version from the same time period) works
> with Firefox 11.

After running updates today and rebooting Ubuntu, use of the win/super/hyper key is now working with the 'Global Menu Bar integration' add-on enabled (although the 'Ctrl' appears where 'Win' appeared in the menu shortcut labels).

Thank you for fixing this!  I apologize for my ignorance in this matter, but is there a specific stable release version we can expect this fix to make it into?  Version 17?
Comment 105 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-08-17 03:25:48 PDT
https://developer.mozilla.org/en-US/docs/XUL/key
https://developer.mozilla.org/en-US/docs/XUL_Tutorial/Keyboard_Shortcuts
Comment 106 David von Oheimb 2012-09-05 10:16:48 PDT
Sigh, this still does not work for me with firefox-16.0b2 of 04-Sep-2012 13:55
even with default settings and safe-mode.
For instance, Meta/Win-C does not copy text to the clipboard 
(as I prefer to have, for simulating Mac OS keyboard bindings).

I'm using default settings of altwin (i.e., Super for Win,
and according to "setxkbmap -print", nothing special for xkb_symbols).
The keycode reported by Firefox for the win key is 91 as usual I guess.
(BTW, if I set altwin(left_meta_win) or altwin(meta_win), Firefox reports keycode=0 !)

xev reports:
KeyRelease event, serial 33, synthetic NO, window 0xb000001,
    root 0xae, subw 0x0, time 1343062756, (131,106), root:(180,200),
    state 0x40, keycode 133 (keysym 0xffeb, Super_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

Neither ui.key.accelkey=91 nor ui.key.accelkey=224 has the desired effect.
ui.key.disable_meta_if_flag_shared appears irrelevant, right?

What is wrong here? So I'm still stuck with Firefox 12 :-(
Comment 107 David von Oheimb 2012-09-05 10:34:46 PDT
(In reply to David von Oheimb from comment #106)
> Sigh, this still does not work for me with firefox-16.0b2 of 04-Sep-2012

Aaargh, I should have used a nightly build, for instance firefox-18.0a1 !

Now ui.key.accelkey=91 works, and ui.key.disable_meta_if_flag_shared is not needed.

When will the fix finally be available on a release version?
On on which one (16, 17, or 18)?
Comment 108 Aaron Solochek 2012-09-05 10:51:57 PDT
That's a good question.  It still doesn't work for me in thunderbird 15 (although it's broken differently in 15 than it is in 13.)  I guess I'll try a nightly of that too, since I'd rather not stay on thunderbird 12.0.1 forever.
Comment 109 David von Oheimb 2012-09-05 11:00:31 PDT
I meanwhile realized with with today's nightly, add-ons are ignored.
The following aurora appears to be a better choice:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-09-05-mozilla-aurora-debug/firefox-17.0a2.en-US.debug-linux-x86_64.tar.bz2
Comment 110 Aaron Solochek 2012-09-05 11:40:32 PDT
I just confirmed that this still doesn't quite right in thunderbird 18.0a1.  When I got to assign a hotkey in my addon, and hit my meta key, it is immediately detected as "meta META."  However if I assign a hotkey to alt-f, then meta-f actually does what alt-f is supposed to do.  So that at least lets me keep working.
Comment 111 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-09-05 17:11:23 PDT
See "Target Milestone" when you want to know the first fixed version of FIXED bug.

This is fixed in Fx17.
Comment 112 Jae-hyeon Park 2012-12-12 07:31:54 PST
I upgraded firefox to version 17.0.1.  I managed to enable access to the menu bar using a Meta key by setting ui.key.menuAccessKey to 224.  For instance, pressing Meta+F brings me to the File menu.

However, I cannot figure out how to set Meta+1, Meta+2,..., as the shortcuts for switching to the first, the second,... tabs.  Also, I cannot get Meta+Enter to open the address in a new tab.  All these worked fine with firefox 12, but with version 17, I have to press Alt+digit and Alt+Enter instead.  Is there a way to change these tab-related modifiers to Meta?

I use a PC keyboard with the following mapping on Linux:

    left Windows key -> Meta_L
    right Windows key -> Meta_R
    left Alt key -> Alt_L
    right Alt key -> Alt_R

I am running xfce4 and the output of xmodmap looks like:

$ xmodmap
xmodmap:  up to 3 keys per modifier, (keycodes in parentheses):

shift       Shift_L (0x32),  Shift_R (0x3e)
lock        Caps_Lock (0x42)
control     Control_L (0x25),  Control_R (0x69)
mod1        Meta_L (0x85),  Meta_R (0x86),  Meta_L (0xcd)
mod2        Num_Lock (0x4d)
mod3        Alt_L (0x40),  Alt_R (0x6c),  Alt_L (0xcc)
mod4        Super_L (0xce),  Hyper_L (0xcf)
mod5        ISO_Level3_Shift (0x5c),  Mode_switch (0xcb)

Note You need to log in before you can comment on or make changes to this bug.