665406 - Support XF86Copy, XF86Paste, XF86Cut, XF86Undo, XF86Redo keysym

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Ginn Chen]

With new Xorg server, Copy, Paste, Cut, Undo, Again on Sun keyboard or other multimedia keyboard return XF86Copy, XF86Paste, XF86Cut, XF86Undo, XF86Redo keysym.
GDK_Copy, GDK_Paste ... for GDK.
Qt::Key_Copy, Qt::Key_Paste ... for Qt.

We should stop using the "strange" F16, F18, F20, F14 keycode.

For backwards compatibility, we can map F16 to Copy, F20 to Cut, etc.
We do not map F12 to Redo because it may confuse PC keyboard users.

Comment 1

[Ginn Chen]

Attachment: draft patch

I removed "#ifdef SOLARIS" for old Sun keysym in nsGtkKeyUtils.cpp in case someone remote Firefox on Linux to Solaris Xserver.
I hope it is OK, I think in most cases F14-F20, SunF36, SunF37 are not used.

I removed Sun keycode conversion in nsQtKeyUtils.cpp.
I think X keysym should not be used here.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 540381 [details] [diff] [review]
draft patch

Review of attachment 540381 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Um... Even if we would take this patch, please use from 0xC1 to 0xCF for the new keycodes because this breaks the patch for bug 630810 and bug 630813.

On the other hand, I'm not sure these keys should cause keydown/keyup events.

Sine such virtual keys are not on Win and Mac, Web application developers can miss to handle of the keys, probably.

I think the better thing is that we should dispatch only content command events at handling the keys. How about you, guys?
http://mxr.mozilla.org/mozilla-central/source/widget/public/nsGUIEvent.h#477

Note that we don't dispatch key events on Windows for special keys on multimedia keyboard.

And Karlt, key handling bug fix like this bug could break bug 630813's patch because it redesigns the structure of key event handling. If I recreate such big patch several times, I could make mistake at merging, so, I'd like you to review it ASAP.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Note that we don't dispatch key events on Windows for special keys on multimedia keyboard.

Ah, my MS multimedia keyboard emulates the general shortcut keys such as Ctrl+C, Ctrl+Z when I press such special keys. But I still don't think we should dispatch the key events with new keycodes.

Comment 5

[Ginn Chen]

I don't really want to make it so complicate.
If it doesn't work for some web app, it's OK.

Probably we can use VK_XF86COPY as the name to make it clear the keycode is only used under X environment.

What do you think?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #5)
> I don't really want to make it so complicate.

Why my idea makes it complicated? I said that I think that we should just dispatch content command event instead of key events and handling them on XUL level. The Windows' implementation is here:
http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#5390

> If it doesn't work for some web app, it's OK.

Why OK? We are developing web application platform. And the different behavior of web applications makes our users unhappy. The compatibility between platforms is very important. I'm working on getting rid of the difference of key event behavior between platforms.

> Probably we can use VK_XF86COPY as the name to make it clear the keycode is
> only used under X environment.

The name isn't matter. And we shouldn't make the difference keycode for every platform's special keycode.

I worry about the count of keycode will reach 256. Then, we cannot use bit field for saving key state by PRUint32[8] (I'm not sure we're using this way actually on current trunk build).

And note that I don't think we should define new keycode for all keys after we implement DOM3 key property. I planed to implement it after I finish current keycode refactoring.

Comment 7

[Ginn Chen]

Attachment: patch v2

Thanks for your advice, here's patch v2.

There's a small different between NS_CONTENT_COMMAND_PASTE and access key.
NS_CONTENT_COMMAND_PASTE will test if clipboard is usable first.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 541586 [details] [diff] [review]
patch v2

I like this better than the old patch. By this approach, we can avoid dispatching special key events which are dispatched only on Linux.

But I should confirm smaug's thought.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 541586 [details] [diff] [review]
patch v2

I wasn't aware of ContentCommandEvent, but after reading that code
this looks ok.

Comment 10

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/8644336b5d40

I have to reverse a little bit to use some XF86XK_ marcos, because the build server doesn't have a recent GTK+.