Use keyboard-types crate #21881

pyfisch · 2018-10-06T17:40:11Z

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent
from the W3C UI Events spec. All keyboard handling now uses the new types.

Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now
recognized in a uniform way.

Updated the winit port.
Updated webdriver integration.

part of #20331

What this PR does:

allow the use non-ASCII keyboards for text input
decouple keyboard event "key" from "code" (key meaning vs location)

What this PR does not do:

completely improve keyboard events send from winit and webdriver
add support for CompositionEvent or IME

Notes:

The winit embedder does not send keyup events for printable keys (this is a regression)
keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.

This change is

highfive · 2018-10-06T17:40:15Z

Heads up! This PR modifies the following files:

@asajeffrey: components/webdriver_server/keys.rs, components/constellation/lib.rs, components/script/textinput.rs, components/script/dom/document.rs, components/script/lib.rs and 7 more
@cbrewster: components/constellation/lib.rs, components/constellation/Cargo.toml, components/constellation/constellation.rs
@jgraham: components/webdriver_server/keys.rs, components/webdriver_server/lib.rs, components/webdriver_server/Cargo.toml
@paulrouget: ports/servo/glutin_app/keyutils.rs, components/compositing/lib.rs, components/constellation/lib.rs, components/servo/lib.rs, ports/servo/non_android_main.rs and 7 more
@KiChjang: components/script/textinput.rs, components/script/dom/document.rs, components/script_traits/lib.rs, components/script/lib.rs, components/script/Cargo.toml and 3 more

highfive · 2018-10-06T17:40:17Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

pyfisch · 2018-10-06T19:02:53Z

@bors-servo try=wpt

bors-servo · 2018-10-06T19:02:59Z

⌛ Trying commit a2b9bed with merge 5567394...


        Auto merge of #21881 - pyfisch:keyboard-types, r=<try>

WIP Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-06T19:07:08Z

💔 Test failed - linux-rel-css

pyfisch · 2018-10-06T19:29:57Z

@bors-servo try=wpt

bors-servo · 2018-10-06T19:30:02Z

⌛ Trying commit 23a84a6 with merge fd00720...


        Auto merge of #21881 - pyfisch:keyboard-types, r=<try>

WIP Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression) * keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-07T05:30:01Z

💥 Test timed out

pyfisch · 2018-10-07T13:14:44Z

Questions for reviewers:

keyboard-types
- naming: KeyState or just State?
- should I remove the CompositionEvent? It is not used yet but will be used for composing keys (and IMEs?).
- ShortcutMatcher currently takes all modifier keys into account. But some keys like NumLock are not usually part of shortcuts and they are locking keys so they will prevent any other shortcuts from working. Which keys are usually used in shortcuts? (Shift, Control, Alt, Meta only?)
- naming: is Key::Character a good name as it contains a String and not a char?
textinput
- BTW selecting text with the arrow keys and shift can cause an assertion failure (select some chars to the left and then press shift+right)
webdriver
- What is this mysterious Return key (\u{E006}) referring to? UI Events key Values only has Enter. (see also w3c/webdriver#1271)
Do all keyboard shortcuts work? (Especially on Mac)
- Control+Shift+3 (capture webrender does not work because the characters emitted by winit for Control+* are messed up

(to be continued ;-) )

When someone reviews this (unfortunately large) PR Id suggest to start with the keyboard-types crate, then continue to the DOM KeyboardEvent and textinput file and last review the embedders winit and webdriver.

components/script/dom/document.rs

components/script/script_thread.rs

pyfisch · 2018-10-07T20:12:04Z

@bors-servo try

bors-servo · 2018-10-07T20:12:10Z

⌛ Trying commit e8d9447 with merge d566217...


        Auto merge of #21881 - pyfisch:keyboard-types, r=<try>

Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression) * keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-07T20:28:34Z

💔 Test failed - android-x86

pyfisch · 2018-10-13T19:15:12Z

@bors-servo try=wpt

bors-servo · 2018-10-13T19:15:17Z

⌛ Trying commit c861942 with merge a8ce352...


        Auto merge of #21881 - pyfisch:keyboard-types, r=<try>

Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression) * keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-13T19:41:57Z

💔 Test failed - linux-rel-wpt

pyfisch · 2018-10-15T10:15:42Z

@bors-servo try=wpt

jdm · 2018-10-15T12:05:24Z

@bors-servo retry

bors-servo · 2018-10-15T12:05:32Z

⌛ Trying commit c861942 with merge 409cd3f...


        Auto merge of #21881 - pyfisch:keyboard-types, r=<try>

Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression) * keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-15T22:05:32Z

💥 Test timed out

jdm · 2018-10-15T22:14:31Z

It was green.

paulrouget · 2018-10-17T09:36:03Z

I'm happy with both this PR and keyboard-types. This is a significant improvement. Thank you.
I haven't had a chance to test on MacOS though because of servo/mozjs#156.

@bors-servo r+

bors-servo · 2018-10-17T09:36:04Z

📌 Commit c861942 has been approved by paulrouget


        Auto merge of #21881 - pyfisch:keyboard-types, r=paulrouget

Use keyboard-types crate Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 What this PR does: * allow the use non-ASCII keyboards for text input * decouple keyboard event "key" from "code" (key meaning vs location) What this PR does not do: * completely improve keyboard events send from winit and webdriver * add support for CompositionEvent or IME Notes: * The winit embedder does not send keyup events for printable keys (this is a regression) * keyboard-types is on crates.io because I believe it to be useful outside of servo. If you prefer I can add a copy in this repo.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21881)

bors-servo · 2018-10-17T09:36:12Z

⌛ Testing commit c861942 with merge 9a0404a...

bors-servo · 2018-10-17T10:24:35Z

☀️ Test successful - android, android-mac, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, status-taskcluster, windows-msvc-dev
Approved by: paulrouget
Pushing 9a0404a to master...


        Bug 1500260 - Use keyboard-types crate. r=emilio

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 This cherry-picks part of servo/servo#21881.


        Bug 1500260 - Use keyboard-types crate. r=emilio

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 This cherry-picks part of servo/servo#21881.

nox · 2018-10-22T08:57:08Z

I'm confused, why are we using a prerelease version of this crate?

pyfisch · 2018-10-22T16:10:40Z

I'm confused, why are we using a prerelease version of this crate?

Because I wanted some feedback on the keyboard-types crate without publishing a new release version for keyboard-types.


        Bug 1500260 - Use keyboard-types crate. r=emilio

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 This cherry-picks part of servo/servo#21881. UltraBlame original commit: 61bdac71c45c07c9f5f6c3adfcd2c9919b93f7fd


        Bug 1500260 - Use keyboard-types crate. r=emilio

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 This cherry-picks part of servo/servo#21881. UltraBlame original commit: 61bdac71c45c07c9f5f6c3adfcd2c9919b93f7fd


        Bug 1500260 - Use keyboard-types crate. r=emilio

Have embedders send DOM keys to servo and use a strongly typed KeyboardEvent from the W3C UI Events spec. All keyboard handling now uses the new types. Introduce a ShortcutMatcher to recognize key bindings. Shortcuts are now recognized in a uniform way. Updated the winit port. Updated webdriver integration. part of #20331 This cherry-picks part of servo/servo#21881. UltraBlame original commit: 61bdac71c45c07c9f5f6c3adfcd2c9919b93f7fd

highfive assigned emilio Oct 6, 2018

highfive added the S-awaiting-review label Oct 6, 2018

pyfisch force-pushed the pyfisch:keyboard-types branch 2 times, most recently from d45aef0 to a2b9bed Oct 6, 2018

highfive added the S-tests-failed label Oct 6, 2018

pyfisch force-pushed the pyfisch:keyboard-types branch from a2b9bed to 23a84a6 Oct 6, 2018

highfive removed the S-tests-failed label Oct 6, 2018

pyfisch changed the title ~~WIP Use keyboard-types crate~~ Use keyboard-types crate Oct 7, 2018

pyfisch force-pushed the pyfisch:keyboard-types branch 2 times, most recently from 6b396de to 7e3a212 Oct 7, 2018

CYBAI reviewed Oct 7, 2018

View changes

components/script/dom/document.rs Outdated Show resolved Hide resolved

components/script/dom/document.rs Outdated Show resolved Hide resolved

components/script/script_thread.rs Outdated Show resolved Hide resolved

pyfisch force-pushed the pyfisch:keyboard-types branch 4 times, most recently from da4d76b to e8d9447 Oct 7, 2018

highfive added the S-tests-failed label Oct 13, 2018

highfive added S-awaiting-merge and removed S-awaiting-review S-tests-failed labels Oct 17, 2018

bors-servo merged commit c861942 into servo:master Oct 17, 2018
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Oct 17, 2018

bors-servo mentioned this pull request Oct 17, 2018

Add background hang monitor #21673

Merged

0 of 5 tasks complete

pyfisch deleted the pyfisch:keyboard-types branch Oct 17, 2018

servo / servo

Use keyboard-types crate #21881

Use keyboard-types crate #21881

pyfisch commented Oct 6, 2018 •

edited

highfive commented Oct 6, 2018

highfive commented Oct 6, 2018

pyfisch commented Oct 6, 2018

bors-servo commented Oct 6, 2018

bors-servo commented Oct 6, 2018

pyfisch commented Oct 6, 2018

bors-servo commented Oct 6, 2018

bors-servo commented Oct 7, 2018

pyfisch commented Oct 7, 2018 •

edited

pyfisch commented Oct 7, 2018

bors-servo commented Oct 7, 2018

bors-servo commented Oct 7, 2018

pyfisch commented Oct 13, 2018

bors-servo commented Oct 13, 2018

bors-servo commented Oct 13, 2018

pyfisch commented Oct 15, 2018

jdm commented Oct 15, 2018

bors-servo commented Oct 15, 2018

bors-servo commented Oct 15, 2018

jdm commented Oct 15, 2018

paulrouget commented Oct 17, 2018

bors-servo commented Oct 17, 2018

bors-servo commented Oct 17, 2018

bors-servo commented Oct 17, 2018

nox commented Oct 22, 2018

pyfisch commented Oct 22, 2018

servo / servo

Join GitHub today

Use keyboard-types crate #21881

Use keyboard-types crate #21881

Conversation

pyfisch commented Oct 6, 2018 • edited

highfive commented Oct 6, 2018

highfive commented Oct 6, 2018

pyfisch commented Oct 6, 2018

bors-servo commented Oct 6, 2018

bors-servo commented Oct 6, 2018

pyfisch commented Oct 6, 2018

bors-servo commented Oct 6, 2018

bors-servo commented Oct 7, 2018

pyfisch commented Oct 7, 2018 • edited

pyfisch commented Oct 7, 2018

bors-servo commented Oct 7, 2018

bors-servo commented Oct 7, 2018

pyfisch commented Oct 13, 2018

bors-servo commented Oct 13, 2018

bors-servo commented Oct 13, 2018

pyfisch commented Oct 15, 2018

jdm commented Oct 15, 2018

bors-servo commented Oct 15, 2018

bors-servo commented Oct 15, 2018

jdm commented Oct 15, 2018

paulrouget commented Oct 17, 2018

bors-servo commented Oct 17, 2018

bors-servo commented Oct 17, 2018

bors-servo commented Oct 17, 2018

nox commented Oct 22, 2018

pyfisch commented Oct 22, 2018

pyfisch commented Oct 6, 2018 •

edited

pyfisch commented Oct 7, 2018 •

edited