New keyboard various fixes

[kchibisov]

The main highlight is modifiers event rework.

[kchibisov]

I'm not sure how PartialEq and Eq should work here. Internally in winit we want to compare by each field, but users might not want such a thing and instead compare just shift/alt, etc.

[ArturKovacs]

I don't use macOS regularly so every time this Option/Alt question comes up, I have to do a small research to remember what this is about.

The way I understand it is the following:

With some keyboard layouts, holding down the Option key and pressing another key allows users to produce special characters. However, some applications have keyboard shortcuts that are bound to Option+another key. With the previous keyboard API, winit only reported the special character and not the original key, which prevented the shortcut from being triggered. This is why we needed option_as_alt, which prevented the option modifier from being applied onto the received character. With the new API winit reports both, the special character (logical_key) and the original character (key_without_modifiers), and therefore there's no need for option_as_alt.

Is this correct?

[kchibisov]

Is this correct?

Yes.

[kchibisov]

I think I don't like the current impl of left/right modifiers and they way the struct is ending up used downstream. I'll rewrite it to be as a separate field instead.

[kchibisov]

Rewrote the way the left/right modifiers are passed, should be good now.

[kchibisov]

I've also removed the deprecated modifiers field.

@daxpedda could you ensure that I haven't broke the web by doing so?

[madsmtm]

MacOS impl looks good, nice to have the OptionAsAlt removed, better that it can now be implemented outside of winit.

[daxpedda]

Web is fine, though I found that if you hold down both left and right of any modifier key and you release only one, it will report that no modifier keys are active, even though you are holding one down. Though this isn't an issue with this PR, it's already one with #2662, I will fix it there.

So this PR LGTM!

[kchibisov]

@daxpedda have you tested the case when you hold the modifier before focusing the browser and simply move the mouse over it?

[daxpedda]

@daxpedda have you tested the case when you hold the modifier before focusing the browser and simply move the mouse over it?

Discussed on Matrix, not applicable here. See #2768.

[kchibisov]

Discussed on Matrix, not applicable here. See #2768.

I think it was re-iterated and your latest commit addresses all of that.

[daxpedda]

Modifiers on Web

Changes

• When losing focus, we report no active modifiers with ModifiersChanged.
• When regaining focus, we can't determine which modifiers are currently active, but we can deduce it from keyboard and mouse events. If focusing happens to coincide with any of those events, it will show up exactly how it does on native.
• Store has_focus in Cell instead of RefCell, it's just a bool.
• Fixed duplicate focus events.

Failed Solution 1

Why we don't "track" modifiers over keyboard and mouse events while the window isn't in focus?
The problem starts with some terminology inconsistencies, a Winit window is usually what a OS describes as a window. That is not the case for Web, on Web a window is just a canvas. We could track mouse events and even keyboard events while the canvas isn't in focus, but not when the actual browser window is not in focus. If we track modifiers while the canvas isn't in focus but the browser window is, modifiers could change while the browser window isn't in focus and we would get stuck modifiers.

Failed Solution 2

Trying to resolve the solution outlined above, we could track modifiers while the canvas isn't in focus but the browser window is, but clear modifiers if the browser window loses focus to avoid getting modifiers stuck when the window loses focus. Unfortunately this doesn't really provide any advantage, because the only way a canvas can regain focus without having lost the browser window focus is with a keyboard or mouse event (unless you do it over scripting, which is becoming increasingly insane).

Conclusion

I wish I could explain any of this better, but this is all very convoluted and the Web APIs are quite limiting.

The conclusion is that there is really no perfect way to have a correct modifier state when regaining the browser window focus, we will have to wait until we get a keyboard or mouse event.

[daxpedda]

Leaving #2817 (comment) here for future reference.

[maroider]

Regarding the left/right modifiers reporting, I personally think the commented out portions of ModifiersState can be used in a sensible manner to support left/right modifiers (i.e. SHIFT = LSHIFT | RSHIFT, but SHIFT can be set regardless of whether we support distinguishing between left and right). The way you've implemented is also serviceable, but I'd like to have a rationale for why this way would be preferred.

---

The wasm changes appear to work as intended, as far as I can tell, so that's nice.

---

Other than that, I only have a couple of nits.

[maroider]

Suggested change

	// The field providing a metadata, it shouldn't be used as a source of truth.
	// The field provides metadata, and shouldn't be used as a source of truth.

nit

[kchibisov]

Regarding the left/right modifiers reporting, I personally think the commented out portions of ModifiersState can be used in a sensible manner to support left/right modifiers (i.e. SHIFT = LSHIFT | RSHIFT, but SHIFT can be set regardless of whether we support distinguishing between left and right). The way you've implemented is also serviceable, but I'd like to have a rationale for why this way would be preferred.

On some platforms you could have modifier change without any key press at all, so left/right can't be determined by any means. Only macOS supports such a thing and only macOS requires this API anyway, because they have broken modifiers input (Option and not AltGr).