Skip to content

Qt swapping ctrl and cmd on OSX #21110

Closed

Description

@tacaswell

There is special casing for ctl / cmd in the Qt backend on OSX

# In OSX, the control and super (aka cmd/apple) keys are switched.
("Key_Control", "control" if sys.platform != "darwin" else "cmd"),
("Key_Meta", "meta" if sys.platform != "darwin" else "control"),

That is making the tests fail.

Looking at a US mac keyboard on the left side I see:

ctrl | option | command

and looking at US windows keyboard I see

ctrl | meta (aka windows) | alt

however that does not tell us what keycodes they actually generate.

We used to implement this logic differently (before we unified the qt backend code)

3.4.3

if sys.platform == 'darwin':
# in OSX, the control and super (aka cmd/apple) keys are switched, so
# switch them back.
SPECIAL_KEYS.update({QtCore.Qt.Key_Control: 'cmd', # cmd/apple key
QtCore.Qt.Key_Meta: 'control',
})

3.2.2

MODIFIER_KEYS = [('super', QtCore.Qt.MetaModifier, QtCore.Qt.Key_Meta),
('alt', QtCore.Qt.AltModifier, QtCore.Qt.Key_Alt),
('ctrl', QtCore.Qt.ControlModifier, QtCore.Qt.Key_Control),
('shift', QtCore.Qt.ShiftModifier, QtCore.Qt.Key_Shift),
]
if sys.platform == 'darwin':
# in OSX, the control and super (aka cmd/apple) keys are switched, so
# switch them back.
SPECIAL_KEYS.update({QtCore.Qt.Key_Control: 'cmd', # cmd/apple key
QtCore.Qt.Key_Meta: 'control',
})
MODIFIER_KEYS[0] = ('cmd', QtCore.Qt.ControlModifier,
QtCore.Qt.Key_Control)
MODIFIER_KEYS[2] = ('ctrl', QtCore.Qt.MetaModifier,
QtCore.Qt.Key_Meta)

This swapping goes all the way back to 5fdd68a (2012).

I think we need to:

  • have a mac user with a mac keyboard veryify that the keys they hit match the keycodes we expect
  • if the keys are right to the human typing then fix the tests (probably by tweaking what the key-codes fed in are)
  • if the keys are wrong to the human typing
    • if they are consistent across mpl versions on osx: gnash our teeth in despair and figure out how we want to fix this (on one hand, we should make sure that the key-codes are consistent across platfroms, on the other hand we do not want to break existing OSX users).
    • if they changed from 3.4 to the default branch, fix the re-mapping

I also have a worry that Qt has started to fix this for us.


FYI: I just ran the latest tests and these are now failing for me and I'm wondering if it has to do with being on a mac? The tests look like they are swapping cmd/ctrl, so I tried swapping the modifier keys on my system and that didn't help.

FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-control] - AssertionError: assert 'cmd+a' == 'ctrl+a'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-alt_control] - AssertionError: assert 'alt+cmd' == 'alt+control'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-control_alt] - AssertionError: assert 'cmd+alt' == 'ctrl+alt'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-modifier_order] - AssertionError: assert 'cmd+alt+ctrl+á' == 'ctrl+alt+super+á'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-backspace_mod] - AssertionError: assert 'cmd+backspace' == 'ctrl+backspace'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-control] - AssertionError: assert 'cmd+a' == 'ctrl+a'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-alt_control] - AssertionError: assert 'alt+cmd' == 'alt+control'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-control_alt] - AssertionError: assert 'cmd+alt' == 'ctrl+alt'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-modifier_order] - AssertionError: assert 'cmd+alt+ctrl+á' == 'ctrl+alt+super+á'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-backspace_mod] - AssertionError: assert 'cmd+backspace' == 'ctrl+backspace'

It looks like these may be getting skipped on the macos-latest CI?

SKIPPED [22] lib/matplotlib/tests/test_backend_qt.py:168: No usable Qt bindings

Originally posted by @greglucas in #20868 (comment)

Activity

  1. added this to the v3.5.0 milestone on Sep 17, 2021
  2. added
    Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
    on Sep 17, 2021
  3. dstansby commented on Sep 18, 2021

    @dstansby
    Member

    https://doc.qt.io/qt-5/qt.html#Modifier-enum says:

    Note: On macOS, the CTRL value corresponds to the Command keys on the keyboard, and the META value corresponds to the Control keys.

    so I think the failing tests make sense; as an example, with qt_key = 'Key_A' andqt_mods = ['ControlModifier'] we should be identifying this as ctrl+A, apart from on macOS where ControlModifier means the Command key has been pressed. So the solution seems to be change the expected answer in the tests if run on macOS.

  4. tacaswell commented on Sep 20, 2021

    @tacaswell
    MemberAuthor

    So the solution seems to be change the expected answer in the tests if run on macOS.

    Expected answer or input? I would lean towards the input side as what we want is "the user hit the key that produces ctrl in our output" which from a user point of view is what we want to preserve.

  5. timhoffm commented on Sep 20, 2021

    @timhoffm
    Member

    Ctrl on Mac is something different than Ctrl on other platforms, it just happens to have the same name. On a Mac, the equivalent to Ctrl is Cmd (e.g. Cmd+C is copy).

    Qt as a cross-platform framework has decided to map this internally, so that Cmd+C will still give the Qt::CTRL modifier, which is reasonable because this way the application code does not have to distinguish between platforms.

    Without checking, I think our current behavior is

    • on Mac: Cmd-C -> Qt::CTRL+Qt::Key_C -> 'cmd-c'
    • on other: Ctrl-C -> Qt::CTRL+Qt::Key_C -> 'ctrl-c'

    Since test_backend_qt.py::test_correct_key entries at the Qt level, we have to adapt the expected outputs to make the tests pass.

    A second question is whether this native remapping is reasonable or if we should behave like Qt. If platform independence is desireable, we could introduce a rcParam that allows to configure that. We cannot switch hard without breaking user code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    OS: AppleRelease criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions