Add support for Jitsi screensharing in electron app #1355
Conversation
| if (event.data.widgetAction === 'jitsi_iframe_loaded') { | ||
| const iframe = this.refs.appFrame.contentWindow | ||
| .document.querySelector('iframe[id^="jitsiConferenceFrame"]'); | ||
| PlatformPeg.get().setupScreenSharingForIframe(iframe); |
There was a problem hiding this comment.
this method should exist in all platforms, so impl in BasePlatform as a no-op so other platforms inherit it
There was a problem hiding this comment.
If it did, you wouldn't need the isElectron check above :)
There was a problem hiding this comment.
@dbkr - which is preferred? Implement the method in all platforms and just leave it as a stub in every platform other than electron? I would guess so. Alternatively checking if the function exists and is a function could be a compromise...?
There was a problem hiding this comment.
@dbkr says the former.
| event.origin = event.originalEvent.origin; | ||
| } | ||
|
|
||
| if (this.state.widgetUrl.indexOf(event.origin) === -1) { |
There was a problem hiding this comment.
Is there a reason this looks for the widget url anywhere rather than just at the start? Granted it it seems unlikely that you'd have a malicious origin hanging out in the widget url somewhere, but still.
There was a problem hiding this comment.
/me points at !this.state.widgetUrl.startsWith(event.origin)
| if (event.data.widgetAction === 'jitsi_iframe_loaded') { | ||
| const iframe = this.refs.appFrame.contentWindow | ||
| .document.querySelector('iframe[id^="jitsiConferenceFrame"]'); | ||
| PlatformPeg.get().setupScreenSharingForIframe(iframe); |
There was a problem hiding this comment.
If it did, you wouldn't need the isElectron check above :)
|
@superdump please can you pick up the review bits & bobs here so we can merge it? O:-) |
The method platform method is instead stubbed on all other platforms.
The event origin should be at the beginning of the URL.
No description provided.