43433 - Meta modifier doesn't work on Solaris, linux [*nix]

Status: RESOLVED FIXED

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

Description

[Axel Hecht]

On Sun Solaris, keybindings use the Meta modifier instead of the Alt, which
is reported by the menus.

Tested on CVS from Jun 21 2000

Axel

Comment 1

[Eric Pollmann]

Triaging Clayton's buglist:

I think this is a XUL thing, so reassigning to the default owner.  This code 
looks interesting:

nsMenuBarListener::KeyPress()

Especially:
#if !defined(XP_UNIX) || defined(NTO)

Unfortunately, because of this, I don't know where the UNIX menu events are 
handled though.  :|  Anyway, off to the gurus...

Comment 2

[Akkana Peck]

Adding hyatt -- if he doesn't still own menu access keys, he'll know who does.

Comment 3

[Mike Pinkerton (not reading bugmail)]

if someone wants to go in and patch it, the code in 
nsMenuFrame::BuildAcceleratorText() just ignores meta. That's a good place to 
start.

does this happen on linux too or just solaris? adding helpwanted, pushing out.

Comment 4

[Raghu Sakleshpur]

Verified to exist on Linux with M16 distribution.

[e-mail]

Comment 5

[Axel Hecht]

I take it that this is SPARC linux?
Changing OS to All, leaving platform on Sun.

Is there any other hardware with a META key that should be tested?

Axel

Comment 6

[Rich Burridge]

Actually it's running on Linux (RedHat 6.mumble), and displaying back 
onto a Sun workstation. Hence the meta key.

Comment 7

[Akkana Peck]

Last time I checked, gtk code wasn't even setting the meta bit, so key bindings
send from nsGtkEventHandler.cpp weren't checking it since it was always wrong
coming from gtk, and nobody ever spent the time to figure out how to get the
real status of the meta bit.

If someone wanted to get meta working in nsGtkEventHandler.cpp, the rest would
probably be trivial.

Comment 8

[Rich Burridge]

An engineer (Ann Adamcik) in my group I believe has a fix for this 
problem. I've asked her to add herself to the cc: and attach a diff for
folks to test out.

Comment 9

[Rich Burridge]

Changing the platform to Solaris so that we pick this up in our bug queries.

Comment 10

[Ann Adamcik]

Attachment: Partial fix - get the meta key recognized as a modifier

Comment 11

[Ann Adamcik]

I think I've got part of this fixed.  The meta key is now being recognized as
a modifier in nsGtkEventHandler.cpp (see attachment).  Next is to get the 
appropriate menu actions to happen with meta as with alt...

Comment 12

[Akkana Peck]

Thanks, Ann!

I have a fix in my tree to make the menu and accelerator keys configurable.  I
was on vacation last week, so I'll have to integrate the fix with whatever
happened in those files in the past week (the fix ranges over several different
parts of the tree), but I expect to get it checked in sometime this week.  If
you have meta working, we should have all parts of the puzzle soon!

Comment 13

[Akkana Peck]

I checked in my fix for configurable modifier bindings.  I noticed that in
nsMenuFrame::BuildAcceleratorText, in the Meta case it returns the string
"Ctrl".  Nobody seemed to know why that was, but I didn't change it.  I did
check with a Mac xpfe person, and verified that the Mac (which uses meta as an
accelerator) does not go through this path.  So it should be safe to change that
Ctrl to Meta if that fixes the strings on Sun.  Try it and let me know.

Meanwhile, to set the accelerator key to meta, you'll probably want the pref:
pref("ui.key.acceleratorKey", 224);
(see my recent posting to mozilla-unix, or the last comment in bug 22515).

I'm going to take this from pinkerton, but keep it helpwanted/future since I
probably won't have time to test alternate Sun keybindings.  But I'll be glad to
code review any fixes you want to contribute, or provide assistance in getting
this working.

Comment 14

[Akkana Peck]

I just checked in a more complete patch that fixes some other problems with
mnemonics, menu access keys, accelerator keys, etc.

Comment 15

[Akkana Peck]

Have Sun people been testing Ann's patch?  I just realized that you might be
waiting for me to check this in (for some reason I thought someone from Sun was
going to check it in).  Linux doesn't mod4 or GDK_Meta_L, or at least, I can't
find a key for which it sets this (I would have assumed the Windows keys, but
they get treated as a regular key, not a modifier, except that they send a null
keysym and string) so I can't test, but the code looks fine, with one exception:
I assume you meant the second 
  { NS_VK_META,       GDK_Meta_L },
line to read
  { NS_VK_META,       GDK_Meta_R },
instead, right?

Anyway, someone please clarify whether this works right on Sun and whether you
want me to check this in, and we'll get Brendan's approval and do so.

Comment 16

[Ann Adamcik]

Attachment: Minor updates to the previous patch

Comment 17

[Ann Adamcik]

I've just tested my changes again with a recent tree.  Debug output shows the 
meta bit being set, at least at this point.  I'm not sure if anything is
checking it, though.

meta-a:

Key Press event: keyCode = 0xe0, char code = ''
Key Press event: keyCode = 0x0, char code = 'a' [meta]

alt-a:

Key Press event: keyCode = 0x12, char code = ''
Key Press event: keyCode = 0x0, char code = 'a' [alt]


Akkana -

It might be easier for you to get approval to check the fix in, though if you'd
rather I do it, that's fine with me.

Comment 18

[Ann Adamcik]

I've done some more testing, with the accelerator key prefs setting, and
meta now works properly.  However, the menu strings are still "Ctrl".  It
doesn't seem to be going through the code in nsMenuFrame::BuildAcceleratorText,
so I'm not sure where the string is actually being set - I'm hunting for it
now.

Also, in Netscape 4.x on Solaris, the menu string is "Alt" but both alt and meta
work as accelerators (i.e. alt-o and meta-o both bring up the open dialog, but
the menu string is "Alt-O").  Do we want the same behavior here?

Comment 19

[Rich Burridge]

As it currently stands, meta-key accellerators don't work,
and the string for the menu is incorrect. We would like
this to be considered for fixing in nsbeta3. The latest attachment
contains a fix for the meta-key problem. Thanks. 

Comment 20

[rubydoo123]

PDT: this inhibits the use of keybindings on unix

Comment 21

[rubydoo123]

milestone

Comment 22

[rubydoo123]

moving over to rtm+

Comment 23

[Ann Adamcik]

With a tree from last week, I've verified that the latest attachment, along 
with the following change, does work as we expect on Solaris.  However, the 
configurable modifier bindings no longer work - 23587 has regressed. It looks 
like nsXULKeyListener.cpp was removed on 9/21 (see bugid 48758). 

I'd like to go ahead and get this fix in, so that when 23587 is fixed again,
the meta key will be usable.

===================================================================
RCS file: /cvsroot/mozilla/layout/xul/base/src/nsMenuFrame.cpp,v
retrieving revision 1.158
diff -c -r1.158 nsMenuFrame.cpp
*** nsMenuFrame.cpp     2000/09/22 04:55:53     1.158
--- nsMenuFrame.cpp     2000/09/27 04:59:50
***************
*** 1423,1429 ****
        {
          case nsIDOMKeyEvent::DOM_VK_META:
            prependPlus = PR_TRUE;
!           aAccelString.AppendWithConversion("Ctrl");
            break;
  
          case nsIDOMKeyEvent::DOM_VK_ALT:
--- 1423,1429 ----
        {
          case nsIDOMKeyEvent::DOM_VK_META:
            prependPlus = PR_TRUE;
!           aAccelString.AppendWithConversion("Meta");
            break;
  
          case nsIDOMKeyEvent::DOM_VK_ALT:

Comment 24

[Akkana Peck]

Re regression of configurable key bindings: the name changed from
"acceleratorKey" to "accelKey" -- maybe that's the problem you're seeing. 
"ui.key.accelKey and ui.key.menuAccessKey are working for me; if they're not
working for you, you should reopen the bug so someone will know there's a
problem.

I'm pulling a solaris tree to test the latest patches.

Comment 25

[Ann Adamcik]

Yep, the new pref name works fine.  I am now able to change the accel key to
alt or to meta.  

Akkana - do you want to put this in, or should I?

Comment 26

[Akkana Peck]

If you have checkin privileges, go for it.  I'm still waiting for my solaris
build (our common build machine disappeared for a while, but it's back now) and
haven't had a chance to try out your fixes.

Comment 27

[Ann Adamcik]

Attachment: Updated patch - includes both the gtk meta key fix, and fixes the menu accel string.

Comment 28

[rubydoo123]

Akkana, please include the required information per the rtm checkin rules

Comment 29

[Akkana Peck]

- Sun users expect to be able to this modifier key.
- Patch is in hand, and should be low risk (shouldn't affect anyone not using
the meta key).
- There is a workaround (use the ctrl or alt keys).

OTOH, we're having trouble getting other people to help test the patch, and
there are some mysterious issues showing up involving meta-arrow keys sometimes
working and sometimes not.  Adding Pike, who might be interested in testing this
patch, and also seeing whether it helps with the meta-arrow issues.

Comment 30

[Axel Hecht]

OK,
I had a chat with akkana, and there seems to be confusion at least on her side.
Sorry for not watching this closely enough, though the comments don't really
show that much confusion.
So I'll rephrase. I pulled the earliest nightly on ftp.mozilla.org,
ftp://ftp.mozilla.org/pub/mozilla/nightly/2000-07-10-08-M17/mozilla-sparc-sun-solaris2.6.tar.gz
to evaluate what was going on at the time I filed the bug.
Recap: all modifiers where stated as ALT in the UI. Using alt didn't work, but 
using the meta key did. That's the bug I filed.
Taking the current cvs, modifiers moved to CRTL, which works on solaris. Only
the menu modifier is still ALT, which is still mistakenly listening to meta.
alt should work, but does not.
I applied Ann's patch, that doesn't change the behaviour.
Having that configurable is nice. But up to know, I have only seen code mixing
up meta and alt.
I tried setting the prefs to 224, which display META in the menus, but alt is
the one to work. HUARRGHH.
If your setting the menu.. pref to 224, the alt key works for selecting menus.

Is this just my machine switching meta <-> alt? No! I just checked on another
one.

So, I hope we are now all sharing the same confusion. And please look into 
54694, as the weird menu opening follows at least my setting the menu key to
alt (which was supposed to mean meta(!?!))

Axel alias Pike

Comment 31

[Akkana Peck]

Yes, I had entirely misinterpreted this bug.  I still like Ann's fix to add
support for the meta key (it would be nice to add that support just on general
principle), but it sounds like the real issue in this bug is that for some
reason, the gtk library, or Xlib, as implemented on solaris interchanges alt and
meta.

If that's indeed the case, it would be easy enough to hack around it (#ifdef
solaris code in the gtk event handler) if we wanted to, but this seems like a
gtk bug and it would be better to know why gtk or Xlib is doing this and whether
it will continue to do it (otherwise a future version might un-do the patch on
the mozilla end).

Axel, what gtk version (gtk-config --version) is on your machine?  Perhaps we
should inspect the source for the relevant gtk version, and see if it does
anything unusual on Solaris.  Or it might be something in the X keycodes coming
from the Sun server.

Comment 32

[Ann Adamcik]

Hmmm, this is confusing.  Here's the behavior I'm seeing (on Solaris7 and 
Solaris9):

	- Menus report Ctrl as the accelerator (or Alt on older builds).
	- Ctrl key (or Alt on older builds) works as accelerator
	- Meta key is ignored (e.g. pressing meta-o results in an 'o' displayed
		in the location field).

Applying the patch and setting ui.key.menuAccelKey to 224 results in the meta
key working and reported in the menus.  Setting the pref to 18 results in the
alt key working and reported in the menus.

I suspect you *do* have alt and meta keys switched somehow.  Can you create a
generic user account and try it?  Has anyone else been able to try this yet?

Comment 33

[rubydoo123]

re-adding need info

Comment 34

[Axel Hecht]

ok, I tried this both over the network and locally, and I get
meta for 18, and 
alt for 224.
I am a mere user, can't help you with creating another one.
My gtk version is 1.2.7, and
38539 May  4 09:47 include/gdk/gdkkeysyms.h
and md5sum
3f6260583cfad89b4e56895a6461ce25 include/gdk/gdkkeysyms.h

This is 
SunOS 5.7 Generic_106541-12 sun4u sparc
or
SunOS 5.7 Generic_106541-05 sun4u sparc

meta and alt work allright in emacs.

Axel

Comment 35

[rubydoo123]

m19

Comment 36

[Phil Peterson]

PDT marking this [rtm-]. Netscape is concentrating on Win32, Mac and Linux.

Comment 37

[rubydoo123]

Vera, you may want to note that solaris is not a supported platform, or is being 
treated as unsupported per pdt comments

Comment 38

[rubydoo123]

moving to future

Comment 39

[Axel Hecht]

Ok, folks, here we go, giving this a FIXED.
My machine moved from Generic_106541-05 to Generic_106541-12 tonight, and now 
meta and alt work as expected. That is, alt is alt is alt.
Sorry for the confusion. Folks on the sunny side of the street, the list of
patches on this machine is long, and probably not best suited for web
publication, if you want the showrev -p, I can mail it to you.

user_pref("ui.key.accelKey", 18);
user_pref("ui.key.menuAccessKey", 17);
makes menus fire on CRTL, and accellerator(sp?) on ALT.

The release notes should mention this behaviour, leaving bug open to cope for
meta issues

Axel

Comment 40

[Axel Hecht]

Sorry for the SPAM,
I thought this bug was done this morning, and didn't check back when I submitted
the form, knowing half of it is still unresolved.
Changing summary to be more precise, please watch out that the release note 
covers the meta-alt problem for old solaris 2.7 installations.

Axel

Comment 41

[Danny Colascione]

anEvent.isMeta = (aGEK->state & GDK_MOD4_MASK) ? PR_TRUE : PR_FALSE;

Using that in nsGtkEventHandler.cpp results in meta working. I'm using gtk-1.2.8
under Linux.

Comment 42

[Danny Colascione]

Actually, it doesn't. not exactly, anyway.

The underlying code now detects meta (and I fixed the list at the beginning of
the file so that it sets keycode 224 to Meta_L and Meta_R), but menu access and
accels *still* won't work. The problem must be at a higher level.

Comment 43

[Danny Colascione]

Sorry, my fault. I forgot to change it in both InitKeyEvent and
InitKey*Press*Event (what's the difference between these two, anyway?)

You also need to add something like the following to the nsKeycodes list:

  { NS_VK_META,       GDK_Meta_L }, 
  { NS_VK_META,       GDK_Meta_R },

Unfortunately, this still doesn't solve the problem completely --- there are
things in Mozilla (like alt+arrow-key navigation) that use bindings="alt", and
so won't work with meta (unless there is a way to tell Mozilla that you want it
to generate alt instead of meta), so, IMHO, it's easier to make meta and alt
synonymous (or, more precisely, to make meta mean alt). 

Comment 44

[Kathleen :Brade]

Danny--can you explain your comment about "...won't work with meta (unless there 
is a way to tell Mozilla that you want it to generate alt instead of meta)"?

I have seen some keybindings which should be defined with "accel" but instead are 
defined with "ctrl" or "meta".  Those should be fixed.  However, I think you are 
talking about another issue.  Maybe provide an example?

Comment 45

[Danny Colascione]

An example, from xpfe/browser/resources/content/unix/platformNavigationBindings.xul:

<key id="goHome" keycode="VK_HOME" observes="Browser:Home" modifiers="alt"/>

I think this was done because, AFAIK, there is only accel. Perhaps an accel2 is
needed for instances where an accelerator key is needed on a key that already
has one, or where it doesn't make sense to use the primary key.

Comment 46

[Akkana Peck]

*** Bug 93701 has been marked as a duplicate of this bug. ***

Comment 47

[sairuh (rarely reading bugmail)]

jrgm, stealing from ya [qa]...

Comment 48

[Marko Macek]

X handling of Meta vs Alt vs Win vs ??? is a mess. 

In reality: Mod1=<mozilla alt> (as in Alt+F) basically always!

Comment 49

[Jay Yan]

Attachment: The latest patch

Up to now, since the crossplatform part has implemented the support for Meta
key, this patch is enough to make Meta key work on Unix, I've tried it on my
Solaris box. works fine. My env is: Solaris 8 with gtk1.2.6

Newer gtk should be more stable to support this Key.

comments or r=?

Thanks


--Jay

Comment 50

[rubydoo123]

removing myself from the cc list

Comment 51

[Kathleen :Brade]

The latest patch looks good to me (r=brade) but I'll let Akkana (or someone
else) do the actually approval / checkbox since she may be aware of an issue
that I'm not aware of.

Comment 52

[Akkana Peck]

Comment on attachment 73192 [details] [diff] [review]
The latest  patch

If it works on Sun and Sun people are okay with it, then it has my r=akkana.  I
can't test it -- can't figure out how to get Linux to recognize it.  (Though my
windowmanager may be blocking it.)

Anyone know which key meta is supposed to be on  on linux, or a howto anywhere
on how to turn it on for people who want to?  I wonder if this works on
Mandrake 8.1 (where they do some funky alt/meta key remap that I was never able
to figure out).  I'd really appreciate if anyone who knows the answer could
update http://www.mozilla.org/unix/customizing.html, or if you don't have
privileges there, post the info in this bug and I'll update the document.

Comment 53

[Christopher Blizzard (:blizzard)]

Comment on attachment 73192 [details] [diff] [review]
The latest  patch

sr=blizzard

Comment 54

[Asa Dotzler [:asa]]

Comment on attachment 73192 [details] [diff] [review]
The latest  patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 55

[Jay Yan]

Hi, akkana:

This patch has got approval, can you help me to check it in, thanks,

-Jay 

Comment 56

[Akkana Peck]

The fix has been checked in.  Thanks, Jay -- I'll let you mark the bug as fixed.

Comment 57

[Akkana Peck]

I forgot this was assigned to me, not Jay.  So I'll go ahead and mark it fixed.

Comment 58

[Jay Yan]

thanks, akk

Comment 59

[Akkana Peck]

*** Bug 134139 has been marked as a duplicate of this bug. ***

Comment 60

[Dawn Endico]

I'm removing this release note item for this bug from the Mozilla 1.0 and
future versions of the release notes because this bug is marked fixed.

Mail me if you think this item should be re-added.

Comment 61

[Frankie]

Spam for bug 129472

Comment 62

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old