70181 - Keyboard shortcuts don't work under BeOS

Status: RESOLVED FIXED

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

Description

[Gregor Rosenauer]

Except for Cmd+W and IIRC Cmd+Q, no shortcut-key seems to work. Mainly Cut, Copy
and Paste, which I miss most.

Comment 1

[Alec Flett]

uh... who is the beOS owner? I don't do Be.

Comment 2

[David Hallowell]

reassigning...
Remember (at least on intel) all BeOS shortcuts use ALT instead on CTRL -
although anyone on BeOS will already know this, but thought I'd put it here for
the record :)

Comment 3

[timeless]

actually, it's configurable, to be alt or ctrl, i don't remember how well we 
support that.

Comment 4

[Gregor Rosenauer]

I tried both Alt and Ctrl (I have Ctrl set as shortcut key, but anyways),
doesn't work. Also Alt/Ctrl+Q does not work (could not check that before;)

Comment 5

[sairuh (rarely reading bugmail)]

i don't have BeOS available, either...

Comment 6

[kurros (Wade Majors)]

To be strict, they can potentially be handled, but the keydown messages are never reaching 
the functions which handle them.

Comment 7

[Aaron Leventhal]

Looking for a BeOS policy/story for the Mozilla programmer's keyboard FAQ & 
reference
www.mozilla.org/projects/ui/accessibility/mozkeyintro.html

Can someone update this with the correct info for BeOS?

This seems like a major bug for BeOS, at least to me.

Comment 8

[timeless]

http://www.be.com/developers/dev_lib/interface_kit.html
among other pages, there should be a lot of stuff in the be bible, when i find 
another url i'll drop it here

Comment 9

[Yannick Koehler]

No more working on Bezilla

Comment 10

[Paul]

I don't think the problem is that the messages aren't getting through.  Only key eventswith modifiers are not working.  Like, in mail/news, I can press "M" to markthe mail as read, and it works.  I don't think we are building the nsKeyEvent that weare forwarding to the event handlers properly.  I need to look at the other platforms,to see if I can find out what we are doing wrong.  A couple of ideas:- We are sending the event without the modifier- We are sending the even "improperly"- We are sending each key event independently, therefor same as the firstNeed to run with debugger ...

Comment 11

[Paul]

I have a patch for this.  The problem is that the key code being used is from
the BView's hook functions
KeyUp and KeyDown.  The value this gets is different depending on the modifiers
being pressed.  Since the 
KEY_UP and KEY_DOWN messages are already being looked at to get the bekeycode,
the patch makes the
code also get the raw_char code.  Then, if any ctrl or alt or shift + ctrl or
alt are pressed when another key
is pressed, I send the raw code, instead of the bytes.  This seems to work for
everyting except shortcuts that
take both a ctrl and shift key.  I am assuming that this problem is a lot deeper
than I can find, unless someone else
sees the problem.  I tried cntrl+shift+l to get the Open Web Location dialog,
and it does send the raw_char for "l"
to the eventhandle, just like it would for the other key sequences, but nothing
happens. 

The patch is below.  Any helpful insights would be appreciated.

Comment 12

[Paul]

Attachment: patch to nsWindow.cpp and nsWindow.h to use the raw_char for key presses with modifiers

Comment 13

[Makoto Hamanaka]

arougthopher, thanks for the patch.
I did once look into this issue but didn't get an answer.
I couldn't get shortcuts work with your patch only, so I did several changes to get it work.

In nsViewBeOS::KeyDown() and nsViewBeOS::KeyUp(),  I need to change like 
this to avoid a crash.
- uint32     args[5]; 
+ uint32     args[6]; 

In nsWindow::OnKeyDown and nsWindow::OnKeyUp, I need to change like this,
to get shortcuts to work.
-  mIsShiftDown   = mod & B_SHIFT_KEY;
-  mIsControlDown = mod & B_CONTROL_KEY;
-  mIsAltDown     = mod & B_COMMAND_KEY;
+  mIsShiftDown   = (mod & B_SHIFT_KEY)?PR_TRUE:PR_FALSE;
+  mIsControlDown = (mod & B_CONTROL_KEY)?PR_TRUE:PR_FALSE;
+  mIsAltDown     = (mod & B_COMMAND_KEY)?PR_TRUE:PR_FALSE;
(I think this is the bug which I can't be able to find at the time of implementing 
handling key events, sadly though.)

Also, to get Alt+whatever key events in nsViewBeOS::KeyDown(), I need to override 
nsWindowBeOS::DispatchMessage() like this :

+// This function calls KeyDown() for Alt+whatever instead of app_server
+void nsWindowBeOS::DispatchMessage(BMessage *msg, BHandler *handler)
+{
+  if (msg->what == B_KEY_DOWN && modifiers() & B_COMMAND_KEY) {
+    BString bytes;
+    if (B_OK == msg->FindString("bytes", &bytes)) {
+      BView *view = this->CurrentFocus();
+      if (view)
+        view->KeyDown(bytes.String(), bytes.Length());
+    }
+  }
+  BWindow::DispatchMessage(msg, handler);
+}
(thanks to toyoshima about suggestioning this code)

And about ctrl+shift, this code worked for me.

  if ((mIsControlDown || mIsAltDown) && rawcode >= 'a' && rawcode <= 'z') {
    if (mIsShiftDown)
      uniChar = rawcode + 'A' - 'a';
    else
      uniChar = rawcode;
  } else {
    if (numBytes == 0) // deal with unmapped key
      return result;

    switch((unsigned char)bytes[0])
    {
      case 0xc8://System Request

Also, I changed Preferences->Debug->Keyboard accelerators to
Accelerator key : 18
Menu Access Key : 17
, then I can use Alt as the shortcut key. Yeah!

Thanks!

Comment 14

[Paul]

I've implemented the changes Makoto recommended.  I had actually done thechange to have the mIsShiftDown, etc. when I first started looking into this, to seeif it was a quick fix.  Then, right before I posted the first patch, removedit, thinking it wasn't needed. Oops! :)  Well, it's there now.As long as no one has any problems, I think this is ready to be committed.  I have notfound any problems or crashes related to these changes.

Comment 15

[Paul]

Attachment: Original patch, plus changes suggested by Makoto

Comment 16

[timeless]

personally i don't see much need for |this->|

can you make this part:
+    virtual PRBool                     OnKeyDown(PRUint32 aEventType, const 
char *bytes, int32 numBytes, PRUint32 mod, PRUint32 bekeycode, int32 rawcode);
+    virtual PRBool                     OnKeyUp(PRUint32 aEventType, const char 
*bytes, int32 numBytes, PRUint32 mod, PRUint32 bekeycode, int32 rawcode);
... line up w/ the stuff above it

can you wrap:
+          printf("nsWindow::OnKeyDown() error: bytes[] has not enough 
chars.\n");
inside an #ifdef DEBUG
?

this is a very bad thing to see:
+        break;
+    }
   }
indentation for a file should be consistent, either 2 spaces or 4. (no tabs in 
mozilla code [people should remove tabs while cleaning up])

nsWindow.cpp
  1 mcafee 1.1 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; 
c-basic-offset: 2 -*-
the modeline says indentations should be 2 spaces, please follow that rule when 
making changes.

w/ those changes i think you can probably take an r= from Ham and someone can 
check your code in (cls might need to rs= but probably not...)

I can do the checkin if you wish. thanks for working on this.

Comment 17

[Paul]

- the |this->| doesn't really hurt anything
- a lot of the code is nsWindow.cpp is not consistently indented.  for that
matter, a lot of the BeOS specific code is not consistently indented.
- sure, i'll make the error message be in debug mode only
- yes, I know the break is not needed.  It was there, I forgot to remove it.
w/ those changes i think you can probably take an r= from Ham and someone can 
check your code in (cls might need to rs= but probably not...)
- as for consistent indentation, see comment above
- all make the changes later this weekend.  it shouldn't matter when, since the
tree is closed for now anyway

Comment 18

[cls]

r=cls.  I replaced the naked printfs with NS_WARNING & #ifdef DEBUG wrappers. 
The modified patch has been checked in.