[Web, keyboard] Locale layout mapping #34625

dkwingsmt · 2022-07-13T02:39:20Z

This PR changes how logical keys are mapped on Web, aligning it with other platforms. Part of flutter/flutter#100456.

This change ensures that non-latin layout can also use letter shortcuts. For example, before this change, on Russian layout, physical key Ctrl-KeyA will produce logical key Ctrl-ф, which will not trigger any common shortcuts. After the change, it will produce logical key Ctrl-A.

Technical Details

The algorithm is as follows: Analyze every key of the current layout,

If a key can produce an alnum under some modifier, then this key is mapped to this alnum.
After the previous step, if some alnum is not mapped, they're mapped to their corresponding key on the US keyboard.
The remaining keys are mapped to the unicode plane according to their produced character.

This algorithm, which we call the benchmark planner, has been applied to all other desktop platforms.

However, we can't simply apply it to Web: Web DOM API does not tell which keyboard layout the user is on, or how the current layout maps keys (there is a KeyboardLayout API that is supported only by Chrome, and explicitly refused by all other browsers). So we have to invent a "blind" algorithm that applies to any layout, while keeping the same result.

Luckily, we're able to fetch a list of "all keyboard layouts" from Microsoft/VSCode repo, and we analyzed all layouts beforehand, and managed to combine the result into a huge code -> key -> result map. You would imagine it being impossible, since different layouts might use the same (code, key) pair for different characters, but in fact such conflicts are surprisingly few, and all the conflicts are mapped to letters. For example, es-linux maps ('KeyY', '←') to y, while de-linux maps ('KeyY', '←') to z.

We can't distinguished these conflicts only by the (code, key) pair, but we can use other information: keyCode. Now, keyCode is a deprecated property, but we really don't see it being removed any time foreseeable. Also, although keyCode is infamous for being platform-dependent, for letter keys it is always equal to the letter character. Therefore such conflicting cases are all mapped to a special value, 0x01, indicating "use keyCode".

Moreover, to reduce the size of the map, we noticed there are certain patterns that can be easily represented by some if statements. These patterns are extracted as the so-called "heuristic mapper". This reduces the map from over 1600 entries to ~450 entries.

To further reduce the package size overhead, this PR also encodes the map into a string that is decoded at run time. This reduces the package size over by ~45% at the cost of code complexity.

Structure

A package called gen_web_locale_keymap is added to tools/, which:

Fetches all layouts from Microsoft/VSCode
Compute the layout map
Generate the resulting map data to web_locale_keymap (see below)
Generate the test cases to web_locale_keymap
- The test cases are an enumeration of all entries (of interest) of all layouts, comparing the result of the resulting mapper class produces the same result as the benchmark planner.

A package called web_locale_keymap is added to third_party, which does the mapping for all printable characters. Since package must be placed in third_party because it uses Microsoft's code (of MIT license).

The SDK rewriter and the license checker are modified to accommodate the new package.

Package size overhead

Compiling a very simple app that listens to key events:

	main.dart.js	Change
Before PR	2048730 bytes
After PR	2053058 bytes	+4328 bytes (0.21%)

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…ngine into web-layout-detection

…b-layout-detection

skia-gold · 2022-10-11T20:25:16Z

Gold has detected about 1 new digest(s) on patchset 20.
View them at https://flutter-engine-gold.skia.org/cl/github/34625

dkwingsmt · 2022-11-28T21:32:32Z

@mdebbar I've sync'd the PR with the latest master, addressing as many of you comments as possible.

I've also made a non-trivial change that compresses the data, which reduces the package size overhead of this PR further (see PR body).

I'm still resolving some CI issues. In the mean time, can you take another look?

lib/web_ui/lib/src/engine/keyboard_binding.dart

gspencergoog · 2022-11-28T22:49:43Z

lib/web_ui/lib/src/engine/keyboard_binding.dart


  final DispatchKeyData performDispatchKeyData;
+  // Whether the current platform is macOS, which affects how certain key events


Is it the case that onMacOS is only used to differentiate things for macOS, or are these keyboard quirks also present on iOS?

Nice find. I've tested it on my iPad and I confirm that they also present on iOS. I'm changing it to onDarwin.

Let me make it a separate PR after this PR.

gspencergoog · 2022-11-28T22:54:46Z

lib/web_ui/lib/src/engine/keyboard_binding.dart

-      result = result + _kCharLowerA - _kCharUpperA;
-    }
-    return result;
+    return isUpperLetter(key.codeUnitAt(0)) && key.length > 1;


Isn't key.length > 1 sufficient? Or can non-named keys have more than one character? Would it be true that key.length > 2 would be enough? Just trying to see if we can avoid checking the code unit range entirely.

That's a good point.

Although I'm not sure if there are any character KeyboardKey.keys that contain multiple JS characters, all keys that LocaleMapping handles are single-character, which is the most important aspect what this function cares about.

I think we can remove this range check for now and revise it when we see a counterexample.

Just did a quick check of the VSCode layouts. It turns out that en-in has a key L̥ which is two characters. Although, my previous algorithm also fails on this.

Do all named keys have a name that is >2 characters?

Pretty sure so. Because all named keys are alnum, therefore having only 1 character conflicts with letter keys.

Also this is the standard for KeyboardEvent.key: https://w3c.github.io/uievents-key/ , which clearly says that key can consist of a letter and a combining code unit (accents).

dkwingsmt · 2022-11-29T10:13:50Z

@gspencergoog I've addressed all your comments. I'd like to add another test for the en-in case, but I'm currently having a problem with felt test. Nevertheless, can you take another look?

mdebbar

LGTM with some minor comments.

If you don't mind, I would like to wait for @eyebrowsoffire's input on the web_locale_keymap_sources thing in web_sdk/BUILD.gn. If it turns out it's needed, then I have to add web_unicode_sources too.

lib/web_ui/lib/src/engine/keyboard_binding.dart

mdebbar · 2022-11-29T14:48:31Z

lib/web_ui/lib/src/engine/keyboard_binding.dart

-    final String? character = logicalKeyIsCharacter ? eventKey : null;
-    final int logicalKey = () {
+    final bool logicalKeyIsCharacter = !_eventKeyIsKeyName(eventKey);
+    final ValueGetter<int> logicalKey = _cached<int>(() {


What's the reason for using _cached here instead of late final:

late final int logicalKey = () { // ... }();

This function might not be evaluated: if the event is a key up event, the logical key is simply the pressed logical key, and this is not needed at all. In this case, I'm hoping not to run the function body, since it's not a trivial amount of work. And as far as I understand, late still always evaluate the value. (I can add a comment to it.)

According to the docs, the late keyword makes the field lazy, which means the right-hand-side won't run until you access the field. At least that's my understanding.

https://dart.dev/null-safety/understanding-null-safety#lazy-initialization

mdebbar · 2022-11-29T14:56:55Z

web_sdk/BUILD.gn

+    inputs =
+        [ "sdk_rewriter.dart" ] + web_ui_sources + web_locale_keymap_sources


Have you tried without web_locale_keymap_sources? I'm curious because in my case, I didn't add sources from the third_party package, and everything seems to be working fine.

Any ideas @eyebrowsoffire ?

web_sdk/test/sdk_rewriter_test.dart

gspencergoog · 2022-11-29T22:07:12Z

lib/web_ui/lib/src/engine/keyboard_binding.dart

+  // Character keys, however, can also have more than 1 code unit: en-in
+  // maps KeyL to L̥/l̥. To resolve this, we check the second code unit.
+  static bool _eventKeyIsKeyName(String key) {
+    return key.length > 1 && key.codeUnitAt(0) < 0x7F && key.codeUnitAt(1) < 0x7F;


Can't you just say key.length > 2? Are there any names that are only two characters?

Yes, such as Fn and F1. And I'm not sure if there are characters of 3 or more code units.

Ahh, OK. LGTM then.

dkwingsmt · 2022-11-29T22:13:44Z

@mdebbar I've applied the fix to BUILD.gn as you said. Those changes are indeed not necessary.

gspencergoog

skia-gold · 2022-11-30T00:10:29Z

Gold has detected about 1 new digest(s) on patchset 91.
View them at https://flutter-engine-gold.skia.org/cl/github/34625

…116243) * 782d3a600 Roll Skia from 6d759de2e5b2 to 24207007711b (3 revisions) (flutter/engine#37968) * e20362343 [Web, keyboard] Locale layout mapping (flutter/engine#34625)

…lutter#116243) * 782d3a600 Roll Skia from 6d759de2e5b2 to 24207007711b (3 revisions) (flutter/engine#37968) * e20362343 [Web, keyboard] Locale layout mapping (flutter/engine#34625)

dkwingsmt added 12 commits June 27, 2022 11:41

WIP

8cf086a

WIP: Better debug log

21c50f0

Merge remote-tracking branch 'origin/main' into web-layout-detection

0883f87

Merge remote-tracking branch 'origin/main' into web-layout-detection

22d2ccf

WIP

3c05c3d

Merge branch 'web-layout-detection' of https://github.com/dkwingsmt/e…

becbf08

…ngine into web-layout-detection

Merge remote-tracking branch 'origin/main' into web-layout-detection

e82bc25

Finish logic (untested)

416f574

Use third_party for layouts

5ed152d

Update gen

cb3b06d

Fix filtering bugs

e7a6473

Format

b88d06f

flutter-dashboard bot added the platform-web Code specifically for the web engine label Jul 13, 2022

dkwingsmt added 9 commits October 10, 2022 12:26

Merge remote-tracking branch 'origin/main' into web-layout-detection

2175a4a

Merge remote-tracking branch 'dkwingsmt/web-layout-detection' into we…

4f1083f

…b-layout-detection

Fix build

0a85ed9

Add scripts

4b0d8e4

Try license

d4920f4

Copyright

249542c

Rename license

3c4cac8

Use case insensitive

84703c7

more lower case

1a7b3d9

dkwingsmt added 7 commits October 11, 2022 17:10

Sort. Migrate to better types.

8798bbc

Fix template

f05dc33

Marshall and unmarshall

f5704c5

Comments

c1a964b

Gen types file

35d42b1

gen json

162e790

engine compilable

0227c4c

dkwingsmt added 3 commits November 28, 2022 11:53

remove gitignore

de98688

Fix nullable

fdd3225

Fix doc

0e32e71

dkwingsmt added 4 commits November 28, 2022 13:47

Merge remote-tracking branch 'origin/main' into web-layout-detection

0d570e2

Add underscore lead. Make everything lower case. Print by line.

a507c33

Better event code encoding

3cfaf33

Signature

c33c9f2

gspencergoog reviewed Nov 28, 2022

View reviewed changes

dkwingsmt added 2 commits November 28, 2022 15:39

Comment. Change dead key mapping to the last.

cedb9bf

Better digit heuristic. Better _eventKeyIsKeyName

fe356c0

mdebbar reviewed Nov 29, 2022

View reviewed changes

en-in test

884e783

gspencergoog reviewed Nov 29, 2022

View reviewed changes

Remove unnecessary build change

3c48e3d

gspencergoog approved these changes Nov 29, 2022

View reviewed changes

Better order

e27af08

dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 30, 2022

auto-submit bot merged commit e203623 into flutter:main Nov 30, 2022

engine-flutter-autoroll mentioned this pull request Nov 30, 2022

Roll Flutter Engine from 25a2ac85f6aa to e20362343564 (2 revisions) flutter/flutter#116243

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 30, 2022

e20362343 [Web, keyboard] Locale layout mapping (flutter/engine#34625)

d4f3a90

dkwingsmt deleted the web-layout-detection branch December 2, 2022 08:10

dkwingsmt mentioned this pull request Aug 24, 2023

Shift-Digit Shortcuts are broken on macOS flutter/flutter#90167

Closed


		final DispatchKeyData performDispatchKeyData;
		// Whether the current platform is macOS, which affects how certain key events

		inputs =
		[ "sdk_rewriter.dart" ] + web_ui_sources + web_locale_keymap_sources

[Web, keyboard] Locale layout mapping #34625

[Web, keyboard] Locale layout mapping #34625

Uh oh!

Conversation

dkwingsmt commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Technical Details

Structure

Package size overhead

Pre-launch Checklist

Uh oh!

skia-gold commented Oct 11, 2022

Uh oh!

dkwingsmt commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Nov 29, 2022

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Nov 29, 2022

Uh oh!

gspencergoog left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skia-gold commented Nov 30, 2022

Uh oh!

Uh oh!

dkwingsmt commented Jul 13, 2022 •

edited

Loading

dkwingsmt commented Nov 28, 2022 •

edited

Loading

dkwingsmt Nov 28, 2022 •

edited

Loading

dkwingsmt Nov 29, 2022 •

edited

Loading

dkwingsmt Nov 29, 2022 •

edited

Loading

gspencergoog left a comment •

edited

Loading