WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
168186
[GTK] Handle caps lock indicator in event modifiers
https://bugs.webkit.org/show_bug.cgi?id=168186
Summary
[GTK] Handle caps lock indicator in event modifiers
Carlos Garcia Campos
Reported
2017-02-12 01:59:15 PST
We are not handling it, causing test fast/events/special-key-events-in-input-text.html to fail.
Attachments
Patch
(13.30 KB, patch)
2017-02-12 02:06 PST
,
Carlos Garcia Campos
mcatanzaro
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2017-02-12 02:06:34 PST
Created
attachment 301299
[details]
Patch
WebKit Commit Bot
Comment 2
2017-02-12 02:08:25 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Michael Catanzaro
Comment 3
2017-02-12 05:24:33 PST
Comment on
attachment 301299
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301299&action=review
> LayoutTests/ChangeLog:9 > + Add platform specific results for fast/events/special-key-events-in-input-text.html. This patch fixes the caps > + lock key case, but we still have different results in the PrintScreen case.
What's different about the PrintScreen case? Do we not ever want to fix it?
> Source/WebCore/platform/PlatformKeyboardEvent.h:169 > + static bool modifiersContainCapsLock(unsigned);
This header is already a mess, so I guess it's OK, but I dislike adding a special function here just for CapsLock. I'd prefer a generic function: static bool modifiersContainModifier(unsigned, PlatformEvent::Modifier) that you would pass PlatformEvent::Modifier::CapsLockKey to. Even if it's currently only used for CapsLock. But your choice.
> Source/WebCore/platform/gtk/PlatformKeyboardEventGtk.cpp:1325 > + // In X11 GDK_LOCK_MASK could be CapsLock or ShiftLock. What GTK+ does in the X11 backend is checking if
This code really confused me. I wrote a decent-length WTF comment to ask you to explain why it would ever be impossible for GTK+ to generate GDK_KEY_Caps_Lock. The key is to understand that it depends on the nutty modifier mapping of the X server. So I'd add that to your comment: "In X11 GDK_LOCK_MASK could be CapsLock or ShiftLock, depending on the modifier mapping of the X server."
Carlos Garcia Campos
Comment 4
2017-02-12 08:44:55 PST
(In reply to
comment #3
)
> Comment on
attachment 301299
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=301299&action=review
> > > LayoutTests/ChangeLog:9 > > + Add platform specific results for fast/events/special-key-events-in-input-text.html. This patch fixes the caps > > + lock key case, but we still have different results in the PrintScreen case. > > What's different about the PrintScreen case? Do we not ever want to fix it?
-INPUT - keydown - false,false,false,false,false - PrintScreen - KeyA - PrintScreen - 44 - 0. Value: "". -INPUT - keyup - false,false,false,false,false - PrintScreen - KeyA - PrintScreen - 44 - 0. Value: "". +INPUT - keydown - false,false,false,false,false - PrintScreen - PrintScreen - PrintScreen - 44 - 0. Value: "". +INPUT - keyup - false,false,false,false,false - PrintScreen - PrintScreen - PrintScreen - 44 - 0. Value: "". I don't think there's anything to fix there, KeyA doesn't look like the right key code for PrintScreen.
> > Source/WebCore/platform/PlatformKeyboardEvent.h:169 > > + static bool modifiersContainCapsLock(unsigned); > > This header is already a mess, so I guess it's OK, but I dislike adding a > special function here just for CapsLock. I'd prefer a generic function: > > static bool modifiersContainModifier(unsigned, PlatformEvent::Modifier) > > that you would pass PlatformEvent::Modifier::CapsLockKey to. Even if it's > currently only used for CapsLock. But your choice.
There are other similar static functions there and currentCapsLockState().
> > Source/WebCore/platform/gtk/PlatformKeyboardEventGtk.cpp:1325 > > + // In X11 GDK_LOCK_MASK could be CapsLock or ShiftLock. What GTK+ does in the X11 backend is checking if > > This code really confused me. I wrote a decent-length WTF comment to ask you > to explain why it would ever be impossible for GTK+ to generate > GDK_KEY_Caps_Lock. The key is to understand that it depends on the nutty > modifier mapping of the X server. So I'd add that to your comment: > > "In X11 GDK_LOCK_MASK could be CapsLock or ShiftLock, depending on the > modifier mapping of the X server."
Ok.
Carlos Garcia Campos
Comment 5
2017-02-12 08:49:49 PST
Committed
r212205
: <
http://trac.webkit.org/changeset/212205
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug