Improve the keybinding input guard using proposed lumino features

[krassowski]

References

• In Unable to use multiple characters with international keyboards on MacOS in 4.1.0 #15744 we learned that users of specific layouts of international keyboards on Mac struggled to type certain characters due to new shortcuts introduced in JupyterLab 4.1.0.
• In Prevent command shortcuts from preventing user input #15790 (JupyterLab 4.1.3) we added a logic which ensures that shortcuts are not executed if user input would be produced by any particular keystroke; this worked...
• But in Ctrl + S triggle browser's "save page", not save notebook in 4.1.3 #15912 we learned that this had more side effects than intended; namely some default browser actions (like saving) were no longer prevented due to change in when the processKeydownEvent is called; this lead to two conclusions:
  • we need to have a way to prevent default action for some keybindings but not others
  • the number of actions which need preventing default is larger than expected and should not be hard-coded
• In New keybinding attribute to indicate shortcuts which prevent default actions lumino#688 (comment) I noted that the current logic will fail for cords; to solve this we need to hold the execution internally and always call processKeydownEvent()
• Optional prevent default and asynchronous hold for keybinding execution lumino#689 implements a way to selectively disable preventDefault and to hold execution until a promise resolves (and to cancel execution if it resolves to false).

This PR requires a new lumino release and may need to wait for 4.2.0; a short-term solution (hotfix) for JupyterLab 4.1.4 is available in #15913.

The CI on this PR is expected to fail until jupyterlab/lumino#689 is merged, released and we bump versions here.

Fixes #15744

Also relates to #4301

Code changes

Use new holdKeyBindingExecution() to improve the logic from #15790 and mark the problematic shortcuts with preventDefault: false.

User-facing changes

• when a keybinding marked with "preventDefault": false conflicts with default browser action such as typing a character, it will not be executed, and it the default browser action will be executed.
• all other keybindings will prevent the default browser action as they used to in JupyterLab <=4.1.2

Backwards-incompatible changes

None

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[krassowski]

This still requires jupyterlab/lumino#689 but I am marking this as ready for review as it can be tested by linking local installation of lumino with jupyterlab/lumino#689 applied.

[krassowski]

How to test it:

1. Without the patch add the following shortcut in JSON Setting Editor → Keyboard Shortcuts:

   {
       "shortcuts": [
           {
               "command": "runmenu:run",
               "selector": "[data-jp-code-runner]",
               "keys": [
                   "Shift ["
               ]
           }
       ]
   }

2. In notebook press Shift + [
3. See that cell gets run (and { does not get typed)
4. Apply the patch by checking out the Optional prevent default and asynchronous hold for keybinding execution lumino#689 lumino branch
5. Build lumino with yarn run build
6. In JupyterLab repo with this branch run jlpm link path/to/lumino/packages/commands
7. Build JupyterLab with jlpm run build
8. Change the setting to (remember to press save icon):

   {
       "shortcuts": [
           {
               "command": "runmenu:run",
               "selector": "[data-jp-code-runner]",
               "keys": [
                   "Shift ["
               ],
               "preventDefault": false
           }
       ]
   }

9. In notebook press Shift + [ again
10. See that { gets typed
11. Change preventDefault to true (and save)
12. In notebook press Shift + [ again
13. See that cell gets run.

[krassowski]

[krassowski]

The failing UI Tests / Visual Regression Tests appear potentially relvant.

[krassowski]

The CI will be fixed once we merge #16078.

[fcollonval]

Thanks @krassowski it looks good to me.

I guess we want to merge first #16078 to ensure the tests are passing for this one too.

[krassowski]

The failures of:

    [jupyterlab] › test/jupyterlab/notebook-search.test.ts:276:7 › Notebook Search › Search in multiple selected cells 

are persistent across three re-runs - I will investigate.

[krassowski]

No that test does not appear relevant:

Actual	Expected

There is 1 frame when the toolbar is not yet there, and one frame when it is half-populated.

However, I wonder if this introduces some more flakiness to the test runs because when we assumed that a command would be executed instantaneously after pressing keys, now it can take a few milliseconds more.

We can improve the logic by attaching keyup which should short-circuit; this may save a few milliseconds in the case if onbeforeinput would not be called.

[fcollonval]

Thanks @krassowski