Skip to content

Fix #19128: webagg reports incorrect values for non-alphanumeric key events on non-qwerty keyboards #19146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Feb 17, 2021
Merged

Fix #19128: webagg reports incorrect values for non-alphanumeric key events on non-qwerty keyboards #19146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9a75c33
Improve webagg support for non-alphanumeric key events on non-qwerty …
Dec 18, 2020
49db5c4
Fix typo on mpl.js
Dec 18, 2020
da8790a
Improve WebAgg backend handling of shift as a modifier
Dec 20, 2020
89ce223
Improve Wx backend handling of modifier keys combinations
Dec 20, 2020
8682cf6
Make meta key name consistent with other backends
Dec 20, 2020
cfe86cc
Add Qt backend support for super key
Dec 22, 2020
f2c65cd
Move Qt backend modifier order for better consistency
Dec 22, 2020
0da9d1e
Make Qt backend consistent when pressing multiple modifiers
Dec 22, 2020
0f9ab5e
Add Caps Lock key to Qt backend SPECIAL_KEY dictionary
Dec 22, 2020
ec45ae6
Document change of specified name for meta key on Qt5 backend
Dec 22, 2020
6a0ea71
Improve Gtk3 backend handling of shift as a modifier
Dec 22, 2020
3d97ec6
Change reference on documentation
Dec 23, 2020
10f0d33
Improve Tk backend handling of shift as a modifier
Dec 23, 2020
d469a24
Fix documentation
Dec 24, 2020
16adba8
Remove stray changes
Dec 24, 2020
2227748
Remove even more stray changes
Dec 24, 2020
57d4c8a
Add to user's event handling documentation a note about inconsistenci…
Dec 27, 2020
84cdf22
Report the changes in the behaviour of the backends
Dec 27, 2020
a545253
Make if condition cleaner on lib/matplotlib/backends/backend_wx.py
tonadev Feb 10, 2021
87012a5
Remove extra space on lib/matplotlib/backends/web_backend/js/mpl.js
tonadev Feb 10, 2021
d19bdd7
DOC: consolidate API change notes
tacaswell Feb 17, 2021
d4ab1b2
FIX: allow many modifiers
tacaswell Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions doc/api/next_api_changes/behavior/17791-AL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
GTK/Tk key name changes
~~~~~~~~~~~~~~~~~~~~~~~
Harmonized key event data across backends
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The different backends wth key translation support, now handle 'shift'
as a sometimes modifier, where the 'shift' prefix won't be added if a
key translation was made.

The *matplotlib.backends.backend_qt5.SPECIAL_KEYS* dictionary contains
keys that do *not* return their unicode name instead they have
manually specified names. The name for *QtCore.Qt.Key_Meta* has
changed to 'meta' to be consistent with the other GUI backends.

The WebAgg backend now handles key translations correctly on non-US
keyboard layouts.


**GTK/Tk key name changes**

The handling of non-ASCII keypresses (as reported in the KeyEvent passed to
``key_press_event``-handlers) in the GTK and Tk backends now correctly reports
Expand Down
29 changes: 29 additions & 0 deletions doc/users/event_handling.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,35 @@ Event name Class Description
'axes_leave_event' `.LocationEvent` mouse leaves an axes
====================== ================ ======================================

.. note::
When connecting to 'key_press_event' and 'key_release_event' events,
you may encounter inconsistencies between the different user interface
toolkits that Matplotlib works with. This is due to inconsistencies/limitations
of the user interface toolkit. The following table shows some basic examples of
what you may expect to receive as key(s) from the different user interface toolkits,
where a comma separates different keys:

============== ============================= ============================== ============================= ============================== ==============================
Key(s) Pressed WxPython Qt WebAgg Gtk Tkinter
============== ============================= ============================== ============================= ============================== ==============================
Shift+2 shift, shift+2 shift, " shift, " shift, " shift, "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What layout are you using? shift+2 is @ on US QWERTY keyboards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Spanish QWERTY keyboard. Sorry! I forgot to consider that fact.

Shift+F1 shift, shift+f1 shift, shift+f1 shift, shift+f1 shift, shift+f1 shift, shift+f1
Shift shift shift shift shift shift
Control control control control control control
Alt alt alt alt alt alt
AltGr Nothing Nothing alt iso_level3_shift iso_level3_shift
CapsLock caps_lock caps_lock caps_lock caps_lock caps_lock
A a a A A A
a a a a a a
Shift+a shift, A shift, A shift, A shift, A shift, A
Shift+A shift, A shift, A shift, a shift, a shift, a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What keyboard has a capital A already? Do you mean with caps lock on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mean with caps lock on.

Ctrl+Shift+Alt control, ctrl+shift, ctrl+alt control, ctrl+shift, ctrl+meta control, ctrl+shit, ctrl+meta control, ctrl+shift, ctrl+meta control, ctrl+shift, ctrl+meta
Ctrl+Shift+a control, ctrl+shift, ctrl+A control, ctrl+shift, ctrl+A control, ctrl+shit, ctrl+A control, ctrl+shift, ctrl+A control, ctrl+shift, ctrl+a
Ctrl+Shift+A control, ctrl+shift, ctrl+A control, ctrl+shift, ctrl+A control, ctrl+shit, ctrl+a control, ctrl+shift, ctrl+a control, ctrl+shift, ctrl+a
F1 f1 f1 f1 f1 f1
Ctrl+F1 control, ctrl+f1 control, ctrl+f1 control, ctrl+f1 control, ctrl+f1 control, ctrl+f1
============== ============================= ============================== ============================= ============================== ==============================

Matplotlib attaches some keypress callbacks by default for interactivity; they
are documented in the :ref:`key-event-handling` section.

Expand Down
21 changes: 13 additions & 8 deletions lib/matplotlib/backends/_backend_tk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ def scroll_event_windows(self, event):
FigureCanvasBase.scroll_event(self, x, y, step, guiEvent=event)

def _get_key(self, event):
key = cbook._unikey_or_keysym_to_mplkey(event.char, event.keysym)
unikey = event.char
key = cbook._unikey_or_keysym_to_mplkey(unikey, event.keysym)

# add modifier keys to the key string. Bit details originate from
# http://effbot.org/tkinterbook/tkinter-events-and-bindings.htm
Expand All @@ -328,25 +329,29 @@ def _get_key(self, event):
# however this is not the case on "darwin", so double check that
# we aren't adding repeat modifier flags to a modifier key.
if sys.platform == 'win32':
modifiers = [(17, 'alt', 'alt'),
(2, 'ctrl', 'control'),
modifiers = [(2, 'ctrl', 'control'),
(17, 'alt', 'alt'),
(0, 'shift', 'shift'),
]
elif sys.platform == 'darwin':
modifiers = [(3, 'super', 'super'),
modifiers = [(2, 'ctrl', 'control'),
(4, 'alt', 'alt'),
(2, 'ctrl', 'control'),
(0, 'shift', 'shift'),
(3, 'super', 'super'),
]
else:
modifiers = [(6, 'super', 'super'),
modifiers = [(2, 'ctrl', 'control'),
(3, 'alt', 'alt'),
(2, 'ctrl', 'control'),
(0, 'shift', 'shift'),
(6, 'super', 'super'),
]

if key is not None:
# shift is not added to the keys as this is already accounted for
for bitmask, prefix, key_name in modifiers:
if event.state & (1 << bitmask) and key_name not in key:
key = '{0}+{1}'.format(prefix, key)
if not (prefix == 'shift' and unikey):
key = '{0}+{1}'.format(prefix, key)

return key

Expand Down
15 changes: 9 additions & 6 deletions lib/matplotlib/backends/backend_gtk3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,17 +213,20 @@ def size_allocate(self, widget, allocation):
self.draw_idle()

def _get_key(self, event):
unikey = chr(Gdk.keyval_to_unicode(event.keyval))
key = cbook._unikey_or_keysym_to_mplkey(
chr(Gdk.keyval_to_unicode(event.keyval)),
unikey,
Gdk.keyval_name(event.keyval))
modifiers = [
(Gdk.ModifierType.MOD4_MASK, 'super'),
(Gdk.ModifierType.MOD1_MASK, 'alt'),
(Gdk.ModifierType.CONTROL_MASK, 'ctrl'),
]
(Gdk.ModifierType.CONTROL_MASK, 'ctrl'),
(Gdk.ModifierType.MOD1_MASK, 'alt'),
(Gdk.ModifierType.SHIFT_MASK, 'shift'),
(Gdk.ModifierType.MOD4_MASK, 'super'),
]
for key_mask, prefix in modifiers:
if event.state & key_mask:
key = '{0}+{1}'.format(prefix, key)
if not (prefix == 'shift' and unikey.isprintable()):
key = '{0}+{1}'.format(prefix, key)
return key

def configure_event(self, widget, event):
Expand Down
7 changes: 5 additions & 2 deletions lib/matplotlib/backends/backend_qt5.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
SPECIAL_KEYS = {QtCore.Qt.Key_Control: 'control',
QtCore.Qt.Key_Shift: 'shift',
QtCore.Qt.Key_Alt: 'alt',
QtCore.Qt.Key_Meta: 'super',
QtCore.Qt.Key_Meta: 'meta',
QtCore.Qt.Key_Super_L: 'super',
QtCore.Qt.Key_Super_R: 'super',
QtCore.Qt.Key_CapsLock: 'caps_lock',
QtCore.Qt.Key_Return: 'enter',
QtCore.Qt.Key_Left: 'left',
QtCore.Qt.Key_Up: 'up',
Expand Down Expand Up @@ -66,9 +69,9 @@
# Elements are (Modifier Flag, Qt Key) tuples.
# Order determines the modifier order (ctrl+alt+...) reported by Matplotlib.
_MODIFIER_KEYS = [
(QtCore.Qt.ShiftModifier, QtCore.Qt.Key_Shift),
(QtCore.Qt.ControlModifier, QtCore.Qt.Key_Control),
(QtCore.Qt.AltModifier, QtCore.Qt.Key_Alt),
(QtCore.Qt.ShiftModifier, QtCore.Qt.Key_Shift),
(QtCore.Qt.MetaModifier, QtCore.Qt.Key_Meta),
]
cursord = {
Expand Down
126 changes: 44 additions & 82 deletions lib/matplotlib/backends/backend_webagg_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,92 +27,54 @@

_log = logging.getLogger(__name__)

# http://www.cambiaresearch.com/articles/15/javascript-char-codes-key-codes
_SHIFT_LUT = {59: ':',
61: '+',
173: '_',
186: ':',
187: '+',
188: '<',
189: '_',
190: '>',
191: '?',
192: '~',
219: '{',
220: '|',
221: '}',
222: '"'}

_LUT = {8: 'backspace',
9: 'tab',
13: 'enter',
16: 'shift',
17: 'control',
18: 'alt',
19: 'pause',
20: 'caps',
27: 'escape',
32: ' ',
33: 'pageup',
34: 'pagedown',
35: 'end',
36: 'home',
37: 'left',
38: 'up',
39: 'right',
40: 'down',
45: 'insert',
46: 'delete',
91: 'super',
92: 'super',
93: 'select',
106: '*',
107: '+',
109: '-',
110: '.',
111: '/',
144: 'num_lock',
145: 'scroll_lock',
186: ':',
187: '=',
188: ',',
189: '-',
190: '.',
191: '/',
192: '`',
219: '[',
220: '\\',
221: ']',
222: "'"}
_SPECIAL_KEYS_LUT = {'Alt': 'alt',
'AltGraph': 'alt',
'CapsLock': 'caps_lock',
'Control': 'control',
'Meta': 'meta',
'NumLock': 'num_lock',
'ScrollLock': 'scroll_lock',
'Shift': 'shift',
'Super': 'super',
'Enter': 'enter',
'Tab': 'tab',
'ArrowDown': 'down',
'ArrowLeft': 'left',
'ArrowRight': 'right',
'ArrowUp': 'up',
'End': 'end',
'Home': 'home',
'PageDown': 'pagedown',
'PageUp': 'pageup',
'Backspace': 'backspace',
'Delete': 'delete',
'Insert': 'insert',
'Escape': 'escape',
'Pause': 'pause',
'Select': 'select',
'Dead': 'dead',
'F1': 'f1',
'F2': 'f2',
'F3': 'f3',
'F4': 'f4',
'F5': 'f5',
'F6': 'f6',
'F7': 'f7',
'F8': 'f8',
'F9': 'f9',
'F10': 'f10',
'F11': 'f11',
'F12': 'f12'}


def _handle_key(key):
"""Handle key codes"""
code = int(key[key.index('k') + 1:])
value = chr(code)
# letter keys
if 65 <= code <= 90:
if 'shift+' in key:
"""Handle key values"""
value = key[key.index('k') + 1:]
if 'shift+' in key:
if len(value) == 1:
key = key.replace('shift+', '')
else:
value = value.lower()
# number keys
elif 48 <= code <= 57:
if 'shift+' in key:
value = ')!@#$%^&*('[int(value)]
key = key.replace('shift+', '')
# function keys
elif 112 <= code <= 123:
value = 'f%s' % (code - 111)
# number pad keys
elif 96 <= code <= 105:
value = '%s' % (code - 96)
# keys with shift alternatives
elif code in _SHIFT_LUT and 'shift+' in key:
key = key.replace('shift+', '')
value = _SHIFT_LUT[code]
elif code in _LUT:
value = _LUT[code]
if value in _SPECIAL_KEYS_LUT:
value = _SPECIAL_KEYS_LUT[value]
key = key[:key.index('k')] + value
return key

Expand Down
13 changes: 8 additions & 5 deletions lib/matplotlib/backends/backend_wx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ class _FigureCanvasWxBase(FigureCanvasBase, wx.Panel):
wx.WXK_CONTROL: 'control',
wx.WXK_SHIFT: 'shift',
wx.WXK_ALT: 'alt',
wx.WXK_CAPITAL: 'caps_lock',
wx.WXK_LEFT: 'left',
wx.WXK_UP: 'up',
wx.WXK_RIGHT: 'right',
Expand Down Expand Up @@ -718,11 +719,13 @@ def _get_key(self, event):
else:
key = None

for meth, prefix in (
[event.AltDown, 'alt'],
[event.ControlDown, 'ctrl'], ):
if meth():
key = '{0}+{1}'.format(prefix, key)
for meth, prefix, key_name in (
[event.ControlDown, 'ctrl', 'control'],
[event.AltDown, 'alt', 'alt'],
[event.ShiftDown, 'shift', 'shift'],):
if meth() and key_name != key:
if not (key_name == 'shift' and key.isupper()):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it just gives up on non-ascii (?!) base on the if...elif...else just above this? Am I reading that right?

I think fixing that is out of scope for this PR (to keep it from getting to big, small PRs are easier to review and get in).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

key = chr(keyval)
# wx always returns an uppercase, so make it lowercase if the shift
# key is not depressed (NOTE: this will not handle Caps Lock)
if not event.ShiftDown():
    key = key.lower()

Thanks to this fragment of code, we get a lowercase letter if shift is not pressed. If it's, then we get an uppercase letter.
Then, the following line of code makes sure we don't get shift+A and rather get shift, enter and A.

if not (key_name == 'shift' and key.isupper()):

How ever we are still interested on getting shift+2 or shift+] as Wx doesn't translate the keys. Hence only when shift was pressed and the key is an uppercase letter we are interested in not adding the shift prefix. If we don't do this, then there would be one more inconsistency on Wx backend.

key = '{0}+{1}'.format(prefix, key)

return key

Expand Down
13 changes: 6 additions & 7 deletions lib/matplotlib/backends/web_backend/js/mpl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,29 +642,28 @@ mpl.figure.prototype._key_event_extra = function (_event, _name) {
mpl.figure.prototype.key_event = function (event, name) {
// Prevent repeat events
if (name === 'key_press') {
if (event.which === this._key) {
if (event.key === this._key) {
return;
} else {
this._key = event.which;
this._key = event.key;
}
}
if (name === 'key_release') {
this._key = null;
}

var value = '';
if (event.ctrlKey && event.which !== 17) {
if (event.ctrlKey && event.key !== 'Control') {
value += 'ctrl+';
}
if (event.altKey && event.which !== 18) {
else if (event.altKey && event.key !== 'Alt') {
value += 'alt+';
}
if (event.shiftKey && event.which !== 16) {
else if (event.shiftKey && event.key !== 'Shift') {
value += 'shift+';
}

value += 'k';
value += event.which.toString();
value += 'k' + event.key;

this._key_event_extra(event, name);

Expand Down