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

Push-to-Talk Support for Jitsi #7709

Open
wants to merge 26 commits into
base: develop
from

Conversation

Projects
None yet
5 participants
@anoadragon453
Copy link
Member

commented Nov 15, 2018

Implements the riot-web portion of Push-to-Talk support for Jitsi voice and video calls. The matrix-react-sdk portion is matrix-org/matrix-react-sdk#2280.

Adds a new native node addon, iohook, which is used for listening to shortcuts globally, meaning even when Riot is minimized. As such, iohook needs to be compiled, however like most native node addons, compiled versions for your platform are downloaded during npm install for you. The idea is you should never have to compile your own version unless you are using an unsupported platform. The currently supported platforms are Windows, Mac and Linux, however I am unsure what the support is for *BSD.

In terms of security, iohook is set to stop itself if it is no longer in charge of any shortcuts. It will only enable itself during a Jitsi call or while recording a new shortcut in settings. While recording a new shortcut, iohook will report every keyboard shortcut that is pressed on the keyboard. This is necessary for shortcut recording. However, if Riot is minimized or otherwise somehow removed from focus while recording a shortcut, the recording session will end.

All of the security functionality is controlled by the Main process, with all of the Renderer processes acting like unprivileged clients. Thus, assuming a security flaw like an XSS only gives you access to functions in the Renderer process, no malicious keylogging should be possible.

The code is set up to allow for any global shortcut to be registered, not just those regarding Push-To-Talk functionality. However, the amount of shortcuts that can be registered at one time is currently controlled by const keybindingRegistrationLimit, which is set to 1, so this limit will need to be raised if any future functionality need be added.

@dbkr

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

Ah - so the thing is that our build system for the electron builds doesn't support native dependencies (we do all our builds on OS X and it can't cross-compile). This might require us getting separate windows, mac and linux build boxes for the electron app. It was bound to happen sooner or later, but it might not be fast I'm afraid.

@anoadragon453

This comment has been minimized.

Copy link
Member Author

commented Nov 16, 2018

@dbkr even those that download builds of the native dependencies already like in this case?

@anoadragon453

This comment has been minimized.

Copy link
Member Author

commented Feb 4, 2019

Now that electron Riot builds have become possible on our build infrastructure, this will become active again.

@anoadragon453 anoadragon453 force-pushed the anoadragon453:anoa/jitsi_ptt branch from 726ca5b to 7ea2073 Feb 22, 2019

anoadragon453 added some commits Feb 27, 2019

Merge branch 'develop' into anoa/jitsi_ptt
* develop:
  Update issue templates
  Fix cleanup script not to remove extracted/bundles directories (#8764)
@anoadragon453

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

Changes all done wooooooo.

CI needs to be updated, but if you want to test locally:

Go into riot-web and run npm install:electron. Then:

Linux:

  • download and extract the native node module.
  • place iohook.node in riot-web/electron_app/node_modules/iohook/builds/electron-v69-linux-x64/build/Release/iohook.node.

MacOS:

  • download and extract the native node module.
  • place iohook.node in riot-web/electron_app/node_modules/iohook/builds/electron-v69-darwin-x64/build/Release/iohook.node.

Make sure matrix-react-sdk is built (anoa/jitsi_ptt branch) (and that you've run npm run build at least once in riot-web.

Then subsequent changes just require running npm run build:bundle:dev && npm run electron.

Should all work. CI is passing right now as it doesn't build electron builds. We need to get our CI to build it for each platform basically which will happen with BuildKite.

anoadragon453 added some commits Mar 3, 2019

Merge branch 'develop' into anoa/jitsi_ptt
* develop: (30 commits)
  [matrix] -> Matrix
  Nudge karma to 3.1.2
  set chrome path for travis CI explicitly
  Updated install spinner referenced in #8913
  Add support for localConfig at $appData/config.json. Move electron-config to $appData/electron-config.json
  Translated using Weblate (Hungarian)
  Update to electron 4.0.6
  Translated using Weblate (Spanish)
  Translated using Weblate (Norwegian Bokmål)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Hindi)
  Translated using Weblate (Greek)
  Translated using Weblate (German)
  Translated using Weblate (Finnish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Portuguese (Brazil))
  Translated using Weblate (Czech)
  Translated using Weblate (Swedish)
  Translated using Weblate (Swedish)
  Allow disabling update mechanism
  ...
Merge branch 'develop' into anoa/jitsi_ptt
* develop: (75 commits)
  Support CI for matching branches on forks
  Update yarn.lock
  v1.0.4
  Prepare changelog for v1.0.4
  v1.0.4
  Released js-sdk & react-sdk
  Translated using Weblate (Irish)
  Translated using Weblate (English (United States))
  Added translation using Weblate (Irish)
  Update README.md grammar
  Cross-promote mobile apps
  Fix downstream branch fetching
  Update issue templates
  Declare the officially supported browsers in the README
  Update version number in issue templates
  Use the right CI branch
  Set up BuildKite for Chrome
  Lint doesn't need develop deps
  Remove Travis (CI)
  Add a basic BuildKite :pipeline:
  ...
Merge branch 'develop' into anoa/jitsi_ptt
* develop: (47 commits)
  Translated using Weblate (Romanian)
  Translated using Weblate (Finnish)
  Translated using Weblate (Dutch)
  Added translation using Weblate (Slovenian)
  bump olm version
  Use a different cookie to expire any cookies people may already have
  Step cookie down to 4 hours
  Fix autolaunch setting appearing toggled off
  Expire mobile guide cookie after 24 hours
  Don't try to save files the user didn't want to save
  v1.0.6
  Prepare changelog for v1.0.6
  v1.0.6
  Released js-sdk & react-sdk, and bump electron version
  Deleted translation using Weblate (English (United Kingdom))
  Translated using Weblate (Italian)
  Translated using Weblate (Irish)
  Translated using Weblate (Finnish)
  Translated using Weblate (Esperanto)
  Translated using Weblate (English (United Kingdom))
  ...

anoadragon453 added some commits Apr 8, 2019

@murat-aksoy

This comment has been minimized.

Copy link

commented Apr 10, 2019

PTT is just working on electron builds? Can I use it for web UI?

@anoadragon453

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

I'm afraid not, as websites aren't allowed to access keystrokes when the browser is minimized.

You can however use the space bar for push to talk if you have the jitsi window focused.

@ara4n

This comment has been minimized.

Copy link
Member

commented May 19, 2019

what remains here? just getting CI working for desktop? is that actually a hard blocker?

@BloodyIron

This comment has been minimized.

Copy link

commented May 19, 2019

@ara4n @anoadragon453 I too would like to know ;)

PTT FTW PLS! :D

@anoadragon453

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.