Closed Bug 1563206 Opened 4 months ago Closed 24 days ago

Add temporary support for native key events for gutenberg

Categories

(Remote Protocol :: Input, task, P3)

Tracking

(firefox71 fixed)

RESOLVED FIXED
Tracking Status
firefox71 --- fixed

People

(Reporter: jdescottes, Assigned: maja_zf)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

See Bug 1563205 for the overall issue.

In scope of the gutenberg test suite support, we will identify a shortlist of shortcuts that need to be supported, and will implement them directly in our Input domain to increase coverage.

The goal of this bug is just to be a temporary solution to fix gutenberg tests until EventUtils is improved in Bug 1563205.

(if by any chance Bug 1563205 is resolved sooner than this one, then we should close it as a duplicate, but we assume Bug 1563205 will be non-trivial)

Check how many tests are impacted

Flags: needinfo?(jdescottes)
Priority: P2 → P3

Julian, are you planning on doing this bug or can I reassign it?

Hi David, feel free to reassign!

Flags: needinfo?(jdescottes)
Assignee: nobody → mjzffr

It's worth noting that Puppeteer doesn't emulate native shortcuts (i.e. shortcuts handled by OS, not Chrome). The gutenberg tests are working around this for Cmd+A by synthesizing the necessary events with EventTarget.dispatchEvent

gutenberg tests use a helper, pressKeyWithModifier, to dispatch shortcuts, and the first parameter is often a generic alias for common combinations on different platforms:

    primary: ( _isApple ) => _isApple() ? [ COMMAND ] : [ CTRL ],
    primaryShift: ( _isApple ) => _isApple() ? [ SHIFT, COMMAND ] : [ CTRL, SHIFT ],
    primaryAlt: ( _isApple ) => _isApple() ? [ ALT, COMMAND ] : [ CTRL, ALT ],
    secondary: ( _isApple ) => _isApple() ? [ SHIFT, ALT, COMMAND ] : [ CTRL, SHIFT, ALT ],
    access: ( _isApple ) => _isApple() ? [ CTRL, ALT ] : [ SHIFT, ALT ],

Affected tests

Ignoring primary+A which doesn't touch CDP (see previous comment)

git grep -H pressKeyWithModifier | grep await | grep -v "\'primary\', \'a\'" | sort | uniq

packages/e2e-tests/specs/a11y.test.js:      await pressKeyWithModifier( 'access', 'h' );
packages/e2e-tests/specs/a11y.test.js:      await pressKeyWithModifier( 'ctrl', '~' );
packages/e2e-tests/specs/a11y.test.js:      await pressKeyWithModifier( 'shift', 'Tab' );
packages/e2e-tests/specs/block-deletion.test.js:            await pressKeyWithModifier( 'access', 'z' );
packages/e2e-tests/specs/block-deletion.test.js:            await pressKeyWithModifier( 'shift', 'ArrowUp' );
packages/e2e-tests/specs/block-hierarchy-navigation.test.js:        await pressKeyWithModifier( 'ctrl', '`' );
packages/e2e-tests/specs/block-hierarchy-navigation.test.js:    await pressKeyWithModifier( 'access', 'o' );
packages/e2e-tests/specs/block-switcher.test.js:        await pressKeyWithModifier( 'alt', 'F10' );
packages/e2e-tests/specs/blocks/classic.test.js:        await pressKeyWithModifier( 'shift', 'Tab' );
packages/e2e-tests/specs/blocks/heading.test.js:        await pressKeyWithModifier( 'primary', 'A' );
packages/e2e-tests/specs/blocks/list.test.js:       await pressKeyWithModifier( 'primary', 'm' );
packages/e2e-tests/specs/blocks/list.test.js:       await pressKeyWithModifier( 'primary', 'z' );
packages/e2e-tests/specs/blocks/list.test.js:       await pressKeyWithModifier( 'primaryShift', 'm' );
packages/e2e-tests/specs/blocks/list.test.js:       await pressKeyWithModifier( 'shift', 'Enter' );
packages/e2e-tests/specs/change-detection.test.js:      await pressKeyWithModifier( 'primary', 'S' );
packages/e2e-tests/specs/keyboard-navigable-blocks.test.js: await pressKeyWithModifier( 'ctrl', '`' );
packages/e2e-tests/specs/links.test.js:     await pressKeyWithModifier( 'primary', 'K' );
packages/e2e-tests/specs/links.test.js:     await pressKeyWithModifier( 'primary', 'k' );
packages/e2e-tests/specs/links.test.js:     await pressKeyWithModifier( 'shiftAlt', 'ArrowLeft' );
packages/e2e-tests/specs/multi-block-selection.test.js:     await pressKeyWithModifier( 'shift', 'ArrowDown' );
packages/e2e-tests/specs/multi-block-selection.test.js:     await pressKeyWithModifier( 'shift', 'ArrowRight' );
packages/e2e-tests/specs/multi-block-selection.test.js:     await pressKeyWithModifier( 'shift', 'ArrowUp' );
packages/e2e-tests/specs/multi-block-selection.test.js:     await pressKeyWithModifier( 'shift', 'Enter' );
packages/e2e-tests/specs/navigable-toolbar.test.js:         await pressKeyWithModifier( 'alt', 'F10' );
packages/e2e-tests/specs/plugins/block-icons.test.js:   await pressKeyWithModifier( 'access', 'o' );
packages/e2e-tests/specs/plugins/deprecated-node-matcher.test.js:       await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/plugins/format-api.test.js:        await pressKeyWithModifier( 'shiftAlt', 'ArrowLeft' );
packages/e2e-tests/specs/plugins/templates.test.js:         await pressKeyWithModifier( 'primary', 'A' );
packages/e2e-tests/specs/preview.test.js:       await pressKeyWithModifier( 'shift', 'Home' );
packages/e2e-tests/specs/publish-panel.test.js:     await pressKeyWithModifier( 'shift', 'Tab' );
packages/e2e-tests/specs/rich-text.test.js:     await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/rich-text.test.js:     await pressKeyWithModifier( 'primary', 'i' );
packages/e2e-tests/specs/rich-text.test.js:     await pressKeyWithModifier( 'primary', 'z' );
packages/e2e-tests/specs/rich-text.test.js:     await pressKeyWithModifier( 'shift', 'ArrowLeft' );
packages/e2e-tests/specs/rich-text.test.js:     await pressKeyWithModifier( 'shift', 'Tab' );
packages/e2e-tests/specs/rtl.test.js:       await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/rtl.test.js:       await pressKeyWithModifier( 'shift', 'Enter' );
packages/e2e-tests/specs/shortcut-help.test.js:     await pressKeyWithModifier( 'access', 'h' );
packages/e2e-tests/specs/sidebar.test.js:       await pressKeyWithModifier( 'ctrl', '`' );
packages/e2e-tests/specs/splitting-merging.test.js:     await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/splitting-merging.test.js:     await pressKeyWithModifier( 'primary', 'z' );
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' );
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 1st block.
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 1st paragraph text.
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 2nd block.
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 2nd paragraph text.
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 3rd block.
packages/e2e-tests/specs/undo.test.js:      await pressKeyWithModifier( 'primary', 'z' ); // Undo 3rd paragraph text.
packages/e2e-tests/specs/writing-flow.test.js:          await pressKeyWithModifier( 'alt', 'Backspace' );
packages/e2e-tests/specs/writing-flow.test.js:          await pressKeyWithModifier( 'primary', 'Backspace' );
packages/e2e-tests/specs/writing-flow.test.js:      await pressKeyWithModifier( 'primary', 'ArrowDown' );
packages/e2e-tests/specs/writing-flow.test.js:      await pressKeyWithModifier( 'primary', 'b' );
packages/e2e-tests/specs/writing-flow.test.js:      await pressKeyWithModifier( 'primary', 'i' );
packages/e2e-tests/specs/writing-flow.test.js:      await pressKeyWithModifier( 'shift', 'Enter' );

Most common shortcuts

git grep pressKey | grep await | cut -d ":" -f 2 | sed -e 's/^[[:space:]]*//' | sort | uniq -c | sort -r

  28 await pressKeyWithModifier( 'primary', 'a' ); # doesn't use CDP
  27 await pressKeyWithModifier( 'primary', 'b' );
  18 await pressKeyWithModifier( 'shift', 'Enter' );
  16 await pressKeyWithModifier( 'shiftAlt', 'ArrowLeft' );
  10 await pressKeyWithModifier( 'ctrl', '`' );
   9 await pressKeyWithModifier( 'primary', 'K' );
   7 await pressKeyWithModifier( 'primary', 'm' );
   6 await pressKeyWithModifier( 'primary', 'z' );
   6 await pressKeyWithModifier( 'primary', 'S' );
...

If you look at the log of CDP method calls in the test log (run on macos), you get a slightly different picture of top shortcuts:

29 Shift + ArrowLeft ( "type":"rawKeyDown","modifiers":8,"windowsVirtualKeyCode":37,"code":"ArrowLeft","key":"ArrowLeft","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
27 Meta + b ("type":"rawKeyDown","modifiers":4,"windowsVirtualKeyCode":66,"code":"KeyB","key":"b","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
19 Shift + Enter ("type":"keyDown","modifiers":8,"windowsVirtualKeyCode":13,"code":"Enter","key":"Enter","text":"\r","unmodifiedText":"\r","autoRepeat":false,"location":0,"isKeypad":false)
16 Alt + Shift + ArrowLeft ("type":"rawKeyDown","modifiers":9,"windowsVirtualKeyCode":37,"code":"ArrowLeft","key":"ArrowLeft","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
12 Meta z ("type":"rawKeyDown","modifiers":4,"windowsVirtualKeyCode":90,"code":"KeyZ","key":"z","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
12 Ctrl + BackQuote ("type":"rawKeyDown","modifiers":2,"windowsVirtualKeyCode":192,"code":"Backquote","key":"`","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
11 Meta K or k ("type":"rawKeyDown","modifiers":4,"windowsVirtualKeyCode":75,"code":"KeyK","key":"K","text":"","unmodifiedText":"","autoRepeat":false,"location":0,"isKeypad":false)
...

I've taken the liberty to investigate related issues while working on this. One conclusion is that the temporary solution described in Comment 0 won't be necessary to increase coverage in gutenberg: as far as I can tell, most (if any) tests are not affected by problematic shortcuts.

So far, the patch for this is a whole bunch of tests, and maybe a "temporary solution" for one key combination. I'll clean that up and push it after the weekend.

Attachment #9095059 - Attachment description: Bug 1563206 - Test top key combinations that affect gutenberg; r=ato → Bug 1563206 - Test top key combinations that affect gutenberg; r=#remote-protocol-reviewers
Pushed by mjzffr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1ce1b20dcc20
Test top key combinations that affect gutenberg; r=ato,remote-protocol-reviewers,jdescottes
Pushed by mjzffr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/67c1068e06d4
Test top key combinations that affect gutenberg; r=ato,remote-protocol-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/63cc54f12356
Call editor commands directly to emulate some native key bindings; r=ato,remote-protocol-reviewers
Flags: needinfo?(mjzffr)
Regressions: 1589844
You need to log in before you can comment on or make changes to this bug.