Skip to content
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.

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

Using different forms of uppercase letters can cause wrong key binding to be set #3851

Closed
1 task done
svobs opened this issue Jun 22, 2022 · 3 comments · Fixed by #3887
Closed
1 task done

Using different forms of uppercase letters can cause wrong key binding to be set #3851

svobs opened this issue Jun 22, 2022 · 3 comments · Fixed by #3887

Comments

@svobs
Copy link
Contributor

svobs commented Jun 22, 2022

System and IINA version:

  • macOS 12.4
  • IINA 1.3.0 build 132

Steps to reproduce:

  1. Open IINA, go to Preferences > Key Bindings.
  2. In the current config, make sure there are no existing bindings containing set window-scale.
  3. Add these lines to the current config:
   Shift+F set window-scale 1.0
   F set window-scale 0.5
   Shift+f set window-scale 2.0
  1. Play a video, then hold Shift and type "f"
  2. Also check Video menu

Expected behavior:

  • Video goes to double size
  • Video menu has only one binding for Shift+F, and that's the last one

Actual behavior:

  • Video goes to half size or regular size (usually)
  • Same binding is set in the menu 3 times

Screen Shot 2022-06-21 at 21 40 54

  • MPV does not have this problem.
@svobs
Copy link
Contributor Author

svobs commented Jun 22, 2022

There are 3 different ways to express Shift + <alphabet char>, and it looks like mpv allows all 3 to be set in config files...which is fine...but in the internal logic, when actually using them as key in a map, they need to be converted to a standard form.

@svobs
Copy link
Contributor Author

svobs commented Jun 22, 2022

The issue is caused by the key not getting standardized before being put in the map. But just discovered another symptom.

  • If the key binding doesn't qualify as a menu item key equivalent, then only the "uppercase single-letter" form will actually work. The other two which include the explicit "Shift+" will get ignored.

This is because the bindings which go into the menubar get standardized into the form MacOS expects.

But for non-menubar key bindings, the user's keystroke is converted from the MacOS event using mpvKeyCode(), which will always result in the uppercase single char form. It is then used for a direct lookup in the (non-standardized) map.

@svobs
Copy link
Contributor Author

svobs commented Jul 22, 2022

I've submitted PR #3887 updated #3847 to contain the fix for this.

IINA is in a little bit of an interesting situation because it evolved away from mpv, and relies on a single string-based identifier for all key sequences, which creates the need for a normalized form. I've assembled all the differences I'm currently aware of and attempted to sketch out a set of rules for normalization, as documented in KeyCodeHelper.swift. Here's a reproduction. It may need changes / additions in the future.

Definitions used here:

  • A "key" is just any individual key on a keyboard (including keyboards from different locales around the world).
  • A "keystroke" for our purposes is any combination of up to 4 different keys which are held down simultaneously, of which only one is a
    "regular" (non-modifier) key, and the rest are "modifier keys". Note that currently we don't enforce the restriction on only one regular key.
  • The 4 "modifier keys" include: "Meta" (aka Command), "Ctrl", "Alt" (aka Option), "Shift"
  • A "key sequence" is an ordered list of up to 4 keystrokes. Whereas a "keystroke" is a set of keys typed in parallel, a "key sequence" is a set
    of keystrokes typed serially.

Normal Form Rules [updated 2023-03-18]:

  1. If the key is matches the exact string "default-bindings", assume it is part of the special line default-bindings start,
    and return it as-is.
  2. The input string is parsed as a sequence of up to 4 keystrokes, each of which is separated by the character -.
    Note that - is itself a valid keystroke, so that e.g. this is a valid 4-key sequence: -------.
  3. Each resulting keystroke shall be parsed into up to 4 keys, each of which is separated by the character +.
    Note that the + character is accepted as a valid key, but it is normalized to PLUS.
  4. Each of the 4 modifiers shall be written with the first letter in uppercase and the remaining letters in lowercase.
  5. There always shall be exactly 1 "regular" key in each keystroke, and it is always the last key in the keystroke.
  6. A keystroke can contain between 0 and 3 modifiers (up to 1 of each kind).
  7. The modifiers, if present, shall respect the following order from left to right: Ctrl, Alt, Shift, Meta
    (e.g., Meta+Ctrl+J is invalid, but Ctrl+Alt+DEL is valid).
  8. If the regular key in the keystroke is of the set of characters which have separate and distinct uppercase and lowercase versions, then
    the keystroke shall never contain an explicit "Shift" modifier but instead shall use the uppercase character.
  9. Special names for certain characters:
    # shall be written as SHARP.
    + shall be written as PLUS.
  10. Any remaining special keys not previously mentioned and which have more than one character in their name shall be written in all uppercase.
    (examples: UP, SPACE, PGDOWN, KP_DEL)

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

Successfully merging a pull request may close this issue.

2 participants