search

chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.37k stars 467 forks source link

[Windows] Windowless Keyboard Handling #2597

Open magreenblatt opened 5 years ago

magreenblatt commented 5 years ago

Original report by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).

Current code does not set properly DOM3 attributes for key events when key event sent via SendKeyEvent in windowless mode.

For reproduction issues run cefclient --off-screen-rendering-enabled --url=https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html:

There is exist 4 issues, all shown by using above test page:

(1) DOM keyboard code is not set. Press left shift once. This will generate keydown and keyup events. You can see that DOM3 attributes are not set properly.

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift'     0   false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''

same in stable chrome (or in windowed mode works also well):

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''

(2) DOM key location is not set for all events. Press left shift once. This will generate keydown and keyup events. You can see that DOM3 attributes are not set properly.

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift'     0   false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''

same in stable chrome (or in windowed mode works also well):

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''

(3) DOM key repreat attribute is not set. Press shift longer until it generate autorepeated events.

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift'     0   false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   ''

same in stable chrome (or in windowed mode works also well):

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    true    -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    true    -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    true    -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    true    -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    true    -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''

Note, that repeat attribute is set only for keydown event, all others should always has false even if OS provide this information for other events (like WM_CHAR/keypress).

(4) DOM location sometimes completely wrong (NUMPAD appears for non numpad characters). Press a key.

keyup   0   65 'A'  65 'A'  ✗   ✗   ✗   ✗   'a'     0   false   -   -   'a'
input   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   'a' -   'a'
textInput   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   'a' -   ''
keypress    97 'a'  97 'a'  97 'a'  ✗   ✗   ✗   ✗   'a'     NUMPAD  false   -   -   ''
keydown 0   65 'A'  65 'A'  ✗   ✗   ✗   ✗   'a'     0   false   -   -   ''

same on chrome:

keyup   0   65 'A'  65 'A'  ✗   ✗   ✗   ✗   'a' KeyA    0   false   -   -   'a'
input   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   'a' -   'a'
textInput   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   'a' -   ''
keypress    97 'a'  97 'a'  97 'a'  ✗   ✗   ✗   ✗   'a' KeyA    0   false   -   -   ''
keydown 0   65 'A'  65 'A'  ✗   ✗   ✗   ✗   'a' KeyA    0   false   -   -   ''

This case actually is relied to cefclient, and not for framework. All others are framework.

I will attach PR which should address all of this issues.

magreenblatt commented 5 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).

Added pull request #198 with fixes.

PS: Similar issue #2139, but it was oriented on linux, and holds some comments about windows about (1 - missing dom key codes).

magreenblatt commented 5 years ago

Original comment by Megumi Tomita (Bitbucket: tomykaira_2).

I found similar but different issue on the latest build (06/20/2019 - CEF 75.0.11+gf50b3c2+chromium-75.0.3770.100 / Chromium 75.0.3770.100).

When pressing SHIFT + 1 for !, keydown event must be invoked with DOM3 key !, but the DOM3 key is 1.

cefclient: UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.100 Safari/537.36 

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift'     0   false   -   -   '"!'
keyup   0   49 '1'  49 '1'  ✓   ✗   ✗   ✗   '1'     0   false   -   -   '"!'
input   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   '!' -   '"!'
textInput   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   '!' -   '"'
keypress    33 '!'  33 '!'  33 '!'  ✓   ✗   ✗   ✗   '!'     NUMPAD  false   -   -   '"'
keydown 0   49 '1'  49 '1'  ✓   ✗   ✗   ✗   '1'     0   false   -   -   '"'
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift'     LEFT    false   -   -   '"'

‌

chrome UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.100 Safari/537.36 

keyup   0   16  16  ✗   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   '!'
keyup   0   49 '1'  49 '1'  ✓   ✗   ✗   ✗   '!' Digit1  0   false   -   -   '!'
input   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   '!' -   '!'
textInput   -   -   -   ✗   ✗   ✗   ✗   -   -   -   -   '!' -   ''
keypress    33 '!'  33 '!'  33 '!'  ✓   ✗   ✗   ✗   '!' Digit1  0   false   -   -   ''
keydown 0   49 '1'  49 '1'  ✓   ✗   ✗   ✗   '!' Digit1  0   false   -   -   ''
keydown 0   16  16  ✓   ✗   ✗   ✗   'Shift' ShiftLeft   LEFT    false   -   -   ''
magreenblatt commented 5 years ago

Original comment by Megumi Tomita (Bitbucket: tomykaira_2).

https://bitbucket.org/tomykaira_2/cef/commits/04e61871b78d2abf8f23d6518d51404d0966653a?at=dom_key

This fixes the problem reported above.

magreenblatt commented 5 years ago

Issue #2139 was marked as a duplicate of this issue.

magreenblatt commented 4 years ago

See also event-handling changes in #2727#comment-55864569

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).

CEF master (83) solved some of listed issues, but still has some errors: DOM3 code, location and repeat are not populated. I’m will try to fix this.

magreenblatt commented 4 years ago

Original comment by Daniel Raban (Bitbucket: daniel-cigloo).

#2848/input-text-direction-key-swap-does-not

‌

may be related, can still reproduce with 4103 commit 04d461e (bb)

magreenblatt commented 4 years ago

Original comment by Vladislav (Bitbucket: vmas, GitHub: vmas).

Perhaps we can change the cefclient's code to pass a scan code as CefKeyEvent.native_key_code to match this field to the field of the KeycodeMapEntry structure in Chromium code: https://source.chromium.org/chromium/chromium/src/+/master:ui/events/keycodes/dom/keycode_converter.h;bpv=1;bpt=1;l=37?q=KeycodeMapEntry&ss=chromium&originalUrl=https:%2F%2Fcs.chromium.org%2F. The CefKeyEvent.native_key_code field is currently ony used to get the DOM keyboard code and nowhere else: https://bitbucket.org/chromiumembedded/cef/src/845322b5fb7b8cab2298f29da42db050d1eb17c7/libcef/browser/native/browser_platform_delegate_native_win.cc#lines-442. The KeycodeConverter::NativeKeycodeToDomCode() function in this code expects a scan code with extended flag, not lParam. We can also use ui::GetScanCodeFromLParam(LPARAM) from ui/events/event_utils.h to get scan code.

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).

I’m add https://bitbucket.org/chromiumembedded/cef/pull-requests/336 .

Thanks on input of anyone. This PR do same things like previous, but this time this is very easy changes (thanks for CEF’s internal changes).

Also, i’m propose to document what is native_key_code is and propose stick with LPARAM. lParam already holds some data, specifically if you forward real system events, like cefclient do, then you will get repeating flag automatically. Otherwise we will need add this flag into cef_event_flags_t. It should be fair to add this flag into flags, but in fact keyboard handling too OS specific, so i’m doesn’t see need in this. At least not in this issue.

PS: Now i’m not sure what event flags (EVENTFLAG_IS_KEY_PAD, EVENTFLAG_IS_LEFT and EVENTFLAG_IS_RIGHT ) are really useful, because in practice they derive from WPARAM/LPARAM, which user should provide anyway, so on windows i doesn’t see any reason to have this flags.

magreenblatt commented 4 years ago

Original comment by Vladislav (Bitbucket: vmas, GitHub: vmas).

I believe we need a unified API as much as possible. We'll probably need these flags and a repeat flag for other platforms. If we do not introduce the repeat flag here, then the end result may be that the flags will work differently on different platforms.
I have some changes related to flags: https://bitbucket.org/vmas/cef/branch/osr_sendkey

magreenblatt commented 4 years ago

Original comment by Vladislav (Bitbucket: vmas, GitHub: vmas).

And probably by passing LPARAM we will get a different value in the CefKeyboardHandler::OnKeyEvent() method.

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).

LPARAM was used as native key code by CEF forever (at least by cefclient). So nothing actually changed. I'm only propose document this more clearly.

We still can decide what this is should be scan code in chromium's understanding, but i'm doesn't see big win here. This will be breaking change for any CEF application which was modeled from reference application (cefclient) in past. So instead of fix thing, we may break it even more.

I'm generally want fix only listed issue (missed population of dom3 code, location and repeat flag), and dont want introduce unrelated things, like adding flag which is not used by current implementation.

So, I agree with unifying, and your concerns, and adding flags, but most likely what I'm will not take care about personally.

magreenblatt commented 4 years ago

I would also like to see a unified keyboard API. I think we would need to take a step back and convert CefKeyEvent to something much closer to the Chromium API (e.g. NativeWebKeyboardEvent and ui::KeyEvent). We can then provide helper functions that convert from (message, wParam, lParam) and other common platform representations to the expected Chromium structure. We also need to write unit tests that cover all of the edge cases (non-latin keyboard layout, AltGr, unicode, IME, etc).

See also issue #1750.

magreenblatt commented 3 years ago

Translate additional CEF modifiers to EF_* flags (see issue #2597)

→ <<cset 2be59f6edd07 (bb)>>

magreenblatt commented 5 years ago

Original changes by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).