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

Always translate home into meta+enter #110

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

qbi-geny
Copy link
Contributor

@qbi-geny qbi-geny commented Sep 25, 2024

Description

JIRA: SYSTEM-2682

Home widget button is broken on ARM SaaS which uses a PaaS image, as desktop & paas images handle the home button differently.

First, the Android documentation for the actual mapping between linux keyboard keycodes and Android keycodes tells us:

  • Linux KEY_HOMEPAGE (Qt::Key_HomePage) maps to Android KEYCODE_HOME and is the physical home button (back to "desktop")
  • Linux KEY_HOME (Qt::Key_Home) maps to Android KEYCODE_MOVE_HOME and is

    "Used for scrolling or moving the cursor around to the start of a line or to the top of a list.

Historically, it seems that the desktop image maps KEYCODE_MOVE_HOME to KEYCODE_HOME, in device/genymotion/vbox86/Genymotion_Virtual_Input.kl, so the SaaS web player mapped the home widget to this button.

Now, the PaaS image does not have this mapping (it is not necessary), so the webplayer has an option to translate this button to Meta+Enter, which is a shortcut for the home button.

However, this results in the Paas in Saas configuration being broken, as the SaaS web player sends (ultimately, through webrtcd mapping & redis) the KEYCODE_MOVE_HOME to the PaaS image.

This fix:

  • Removes the option, and always translate home into Meta+Enter as it's always a valid shortcut (tested Android 8-14).
    • This will require removing the option in web-ui.
  • Changes the Keycode associated with the home widget button to KEY_HOMEPAGE, as it is more accurate, even though it ends up being translated. We could later remove the translation, when webrtcd is aware of this keycode.

Fixes SYSTEM-2682

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I've read & comply with the contributing guidelines
  • I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
  • I have made corresponding changes to the documentation (README.md).
  • I've checked my modifications for any breaking changes.

@qbi-geny qbi-geny force-pushed the dev/qbi/SYSTEM-2682/always-translate-home branch 2 times, most recently from 0a0d3a4 to e3433ad Compare September 25, 2024 12:40
@jparez
Copy link
Contributor

jparez commented Sep 30, 2024

The status is "draft", but does it still need to be reviewed ?

@qbi-geny
Copy link
Contributor Author

Yes it is still draft, because so far I haven't managed to validate it works on SaaS.

Meta+enter is always a valid shortcut to the physical home button,
so there is no need for differentiating between desktop+saas and paas.

Note the proper fix would be sending HOMEPAGE and map it to HOME,
but this would require adding the mapping to webrtcd, so we went the
easier route instead.
@qbi-geny qbi-geny force-pushed the dev/qbi/SYSTEM-2682/always-translate-home branch from e3433ad to e08cdd6 Compare September 30, 2024 08:29
@qbi-geny qbi-geny marked this pull request as ready for review September 30, 2024 11:52
@qbi-geny
Copy link
Contributor Author

Finally managed to test it in example web player against staging SaaS ARM when based on #111

@qbi-geny qbi-geny merged commit 0958bfe into main Sep 30, 2024
1 check passed
@qbi-geny qbi-geny deleted the dev/qbi/SYSTEM-2682/always-translate-home branch September 30, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants