From 5b5470ec665445b6d3f29099de1edbe0f19ac04c Mon Sep 17 00:00:00 2001 From: paweldomas Date: Mon, 17 Jul 2017 13:38:46 +0200 Subject: [PATCH 1/5] ref(conference.js): createInitialLocalTracksAndConnect Make 'createInitialLocalTracksAndConnect' not static. --- conference.js | 129 ++++++++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 63 deletions(-) diff --git a/conference.js b/conference.js index d46c43dba..9f1a42fdc 100644 --- a/conference.js +++ b/conference.js @@ -124,68 +124,6 @@ function connect(roomName) { }); } -/** - * Creates local media tracks and connects to room. Will show error - * dialogs in case if accessing local microphone and/or camera failed. Will - * show guidance overlay for users on how to give access to camera and/or - * microphone, - * @param {string} roomName - * @returns {Promise.} - */ -function createInitialLocalTracksAndConnect(roomName) { - let audioAndVideoError, - audioOnlyError, - videoOnlyError; - - JitsiMeetJS.mediaDevices.addEventListener( - JitsiMeetJS.events.mediaDevices.PERMISSION_PROMPT_IS_SHOWN, - browser => - APP.store.dispatch( - mediaPermissionPromptVisibilityChanged(true, browser)) - ); - - // First try to retrieve both audio and video. - let tryCreateLocalTracks = createLocalTracks( - { devices: ['audio', 'video'] }, true) - .catch(err => { - // If failed then try to retrieve only audio. - audioAndVideoError = err; - return createLocalTracks({ devices: ['audio'] }, true); - }) - .catch(err => { - audioOnlyError = err; - - // Try video only... - return createLocalTracks({ devices: ['video'] }, true); - }) - .catch(err => { - videoOnlyError = err; - - return []; - }); - - return Promise.all([ tryCreateLocalTracks, connect(roomName) ]) - .then(([tracks, con]) => { - APP.store.dispatch(mediaPermissionPromptVisibilityChanged(false)); - if (audioAndVideoError) { - if (audioOnlyError) { - // If both requests for 'audio' + 'video' and 'audio' only - // failed, we assume that there is some problems with user's - // microphone and show corresponding dialog. - APP.UI.showDeviceErrorDialog( - audioOnlyError, videoOnlyError); - } else { - // If request for 'audio' + 'video' failed, but request for - // 'audio' only was OK, we assume that we had problems with - // camera and show corresponding dialog. - APP.UI.showDeviceErrorDialog(null, audioAndVideoError); - } - } - - return [tracks, con]; - }); -} - /** * Share data to other users. * @param command the command @@ -553,6 +491,70 @@ export default { * Whether the local participant is the dominant speaker in the conference. */ isDominantSpeaker: false, + + /** + * Creates local media tracks and connects to a room. Will show error + * dialogs in case accessing the local microphone and/or camera failed. Will + * show guidance overlay for users on how to give access to camera and/or + * microphone, + * @param {string} roomName + * @returns {Promise.} + */ + createInitialLocalTracksAndConnect(roomName) { + let audioAndVideoError, + audioOnlyError, + videoOnlyError; + + JitsiMeetJS.mediaDevices.addEventListener( + JitsiMeetJS.events.mediaDevices.PERMISSION_PROMPT_IS_SHOWN, + browser => + APP.store.dispatch( + mediaPermissionPromptVisibilityChanged(true, browser)) + ); + + // First try to retrieve both audio and video. + let tryCreateLocalTracks = createLocalTracks( + { devices: ['audio', 'video'] }, true) + .catch(err => { + // If failed then try to retrieve only audio. + audioAndVideoError = err; + return createLocalTracks({ devices: ['audio'] }, true); + }) + .catch(err => { + audioOnlyError = err; + + // Try video only... + return createLocalTracks({ devices: ['video'] }, true); + }) + .catch(err => { + videoOnlyError = err; + + return []; + }); + + return Promise.all([ tryCreateLocalTracks, connect(roomName) ]) + .then(([tracks, con]) => { + APP.store.dispatch( + mediaPermissionPromptVisibilityChanged(false)); + if (audioAndVideoError) { + if (audioOnlyError) { + // If both requests for 'audio' + 'video' and 'audio' + // only failed, we assume that there is some problems + // with user's microphone and show corresponding dialog. + APP.UI.showDeviceErrorDialog( + audioOnlyError, videoOnlyError); + } else { + // If request for 'audio' + 'video' failed, but request + // for 'audio' only was OK, we assume that we had + // problems with camera and show corresponding dialog. + APP.UI.showDeviceErrorDialog(null, audioAndVideoError); + } + } + + return [tracks, con]; + }); + }, + /** * Open new connection and join to the conference. * @param {object} options @@ -588,7 +590,8 @@ export default { {enableAnalyticsLogging: analytics.isEnabled()}, config) ).then(() => { analytics.init(); - return createInitialLocalTracksAndConnect(options.roomName); + return this.createInitialLocalTracksAndConnect( + options.roomName); }).then(([tracks, con]) => { tracks.forEach(track => { if((track.isAudioTrack() && initialAudioMutedState) From 3926d705adb3bcce5d0b845f7cddae1bdac57d8b Mon Sep 17 00:00:00 2001 From: paweldomas Date: Thu, 29 Jun 2017 19:43:35 +0200 Subject: [PATCH 2/5] feat: add config.startScreenSharing Will try to use screensharing instead of camera video from the beginning. --- conference.js | 402 ++++++++++++++++++++++++++--------------- config.js | 1 + lang/main.json | 2 + modules/UI/UI.js | 55 +++++- service/UI/UIEvents.js | 6 - 5 files changed, 311 insertions(+), 155 deletions(-) diff --git a/conference.js b/conference.js index 9f1a42fdc..b4fdebeef 100644 --- a/conference.js +++ b/conference.js @@ -78,11 +78,6 @@ let connection; let localAudio, localVideo; let initialAudioMutedState = false, initialVideoMutedState = false; -/** - * Indicates whether extension external installation is in progress or not. - */ -let DSExternalInstallationInProgress = false; - import {VIDEO_CONTAINER_TYPE} from "./modules/UI/videolayout/VideoContainer"; /* @@ -498,11 +493,14 @@ export default { * show guidance overlay for users on how to give access to camera and/or * microphone, * @param {string} roomName + * @param {boolean} startScreenSharing - if true should start with + * screensharing instead of camera video. * @returns {Promise.} */ - createInitialLocalTracksAndConnect(roomName) { + createInitialLocalTracksAndConnect(roomName, startScreenSharing) { let audioAndVideoError, audioOnlyError, + screenSharingError, videoOnlyError; JitsiMeetJS.mediaDevices.addEventListener( @@ -513,31 +511,55 @@ export default { ); // First try to retrieve both audio and video. - let tryCreateLocalTracks = createLocalTracks( - { devices: ['audio', 'video'] }, true) - .catch(err => { - // If failed then try to retrieve only audio. - audioAndVideoError = err; - return createLocalTracks({ devices: ['audio'] }, true); - }) - .catch(err => { - audioOnlyError = err; + let tryCreateLocalTracks; - // Try video only... - return createLocalTracks({ devices: ['video'] }, true); - }) - .catch(err => { - videoOnlyError = err; + // FIXME the logic about trying to go audio only on error is duplicated + if (startScreenSharing) { + tryCreateLocalTracks = this._createDesktopTrack() + .then(desktopStream => { + return createLocalTracks({ devices: ['audio'] }, true) + .then(([audioStream]) => { + return [desktopStream, audioStream]; + }) + .catch(error => { + audioOnlyError = error; + return [desktopStream]; + }); + }).catch(error => { + logger.error('Failed to obtain desktop stream', error); + screenSharingError = error; + return createLocalTracks({ devices: ['audio'] }, true); + }).catch(error => { + audioOnlyError = error; + return []; + }); + } else { + tryCreateLocalTracks = createLocalTracks( + {devices: ['audio', 'video']}, true) + .catch(err => { + // If failed then try to retrieve only audio. + audioAndVideoError = err; + return createLocalTracks({devices: ['audio']}, true); + }) + .catch(err => { + audioOnlyError = err; - return []; - }); + // Try video only... + return createLocalTracks({devices: ['video']}, true); + }) + .catch(err => { + videoOnlyError = err; + + return []; + }); + } return Promise.all([ tryCreateLocalTracks, connect(roomName) ]) .then(([tracks, con]) => { APP.store.dispatch( mediaPermissionPromptVisibilityChanged(false)); - if (audioAndVideoError) { - if (audioOnlyError) { + if (audioAndVideoError || audioOnlyError) { + if (audioOnlyError || videoOnlyError) { // If both requests for 'audio' + 'video' and 'audio' // only failed, we assume that there is some problems // with user's microphone and show corresponding dialog. @@ -551,6 +573,18 @@ export default { } } + // FIXME If there was a screen sharing error or the extension + // needs to be installed it will appear on top of eventual + // "microphone error" dialog. That is not great, but currently + // it's pretty hard to chain dialogs since they don't return + // Promises. + if (screenSharingError) { + // FIXME if _handleScreenSharingError will be dealing with + // installing external extension it may close previously + // opened microphone dialog ($.prompt.close(); is called). + this._handleScreenSharingError(screenSharingError); + } + return [tracks, con]; }); }, @@ -591,7 +625,8 @@ export default { ).then(() => { analytics.init(); return this.createInitialLocalTracksAndConnect( - options.roomName); + options.roomName, + config.startScreenSharing); }).then(([tracks, con]) => { tracks.forEach(track => { if((track.isAudioTrack() && initialAudioMutedState) @@ -1197,7 +1232,6 @@ export default { /** * Toggles between screensharing and camera video. - * @param {boolean} [shareScreen] * @param {Object} [options] - Screen sharing options that will be passed to * createLocalTracks. * @param {Array} [options.desktopSharingSources] - Array with the @@ -1221,119 +1255,206 @@ export default { } if (!this._untoggleScreenSharing) { - this.videoSwitchInProgress = true; - let externalInstallation = false; - const didHaveVideo = Boolean(localVideo); - const wasVideoMuted = this.videoMuted; - - return createLocalTracks({ - desktopSharingSources: options.desktopSharingSources, - devices: ['desktop'], - desktopSharingExtensionExternalInstallation: { - interval: 500, - checkAgain: () => { - return DSExternalInstallationInProgress; - }, - listener: (status, url) => { - switch(status) { - case "waitingForExtension": - DSExternalInstallationInProgress = true; - externalInstallation = true; - APP.UI.showExtensionExternalInstallationDialog( - url); - break; - case "extensionFound": - if(externalInstallation) //close the dialog - $.prompt.close(); - break; - default: - //Unknown status - } - } - } - }).then(([stream]) => { - // Stores the "untoggle" handler which remembers whether was - // there any video before and whether was it muted. - this._untoggleScreenSharing - = this._turnScreenSharingOff - .bind(this, didHaveVideo, wasVideoMuted); - DSExternalInstallationInProgress = false; - // close external installation dialog on success. - if(externalInstallation) - $.prompt.close(); - stream.on( - TrackEvents.LOCAL_TRACK_STOPPED, - () => { - // If the stream was stopped during screen sharing - // session then we should switch back to video. - if (this.isSharingScreen){ - this._untoggleScreenSharing - && this._untoggleScreenSharing(); - } - } - ); - return this.useVideoStream(stream); - }).then(() => { - this.videoSwitchInProgress = false; - JitsiMeetJS.analytics.sendEvent( - 'conference.sharingDesktop.start'); - logger.log('sharing local desktop'); - }).catch(err => { - // close external installation dialog to show the error. - if(externalInstallation) - $.prompt.close(); - this.videoSwitchInProgress = false; - - if (err.name === TrackErrors.CHROME_EXTENSION_USER_CANCELED) { - return Promise.reject(err); - } - - // Pawel: With this call I'm trying to preserve the original - // behaviour although it is not clear why would we "untoggle" - // on failure. I suppose it was to restore video in case there - // was some problem during "this.useVideoStream(desktopStream)". - // It's important to note that the handler will not be available - // if we fail early on trying to get desktop media (which makes - // sense, because the camera video is still being used, so - // nothing to "untoggle"). - if (this._untoggleScreenSharing) { - this._untoggleScreenSharing(); - } - - logger.error('failed to share local desktop', err); - - if (err.name === TrackErrors.FIREFOX_EXTENSION_NEEDED) { - APP.UI.showExtensionRequiredDialog( - config.desktopSharingFirefoxExtensionURL - ); - return Promise.reject(err); - } - - // Handling: - // TrackErrors.PERMISSION_DENIED - // TrackErrors.CHROME_EXTENSION_INSTALLATION_ERROR - // TrackErrors.GENERAL - // and any other - let dialogTxt; - let dialogTitleKey; - - if (err.name === TrackErrors.PERMISSION_DENIED) { - dialogTxt = APP.translation.generateTranslationHTML( - "dialog.screenSharingPermissionDeniedError"); - dialogTitleKey = "dialog.error"; - } else { - dialogTxt = APP.translation.generateTranslationHTML( - "dialog.failtoinstall"); - dialogTitleKey = "dialog.permissionDenied"; - } - - APP.UI.messageHandler.openDialog( - dialogTitleKey, dialogTxt, false); - }); + return this._switchToScreenSharing(options); } else { return this._untoggleScreenSharing(); } }, + + /** + * Creates desktop (screensharing) {@link JitsiLocalTrack} + * @param {Object} [options] - Screen sharing options that will be passed to + * createLocalTracks. + * + * @return {Promise.} - A Promise resolved with + * {@link JitsiLocalTrack} for the screensharing or rejected with + * {@link JitsiTrackError}. + * + * @private + */ + _createDesktopTrack(options = {}) { + let externalInstallation = false; + let DSExternalInstallationInProgress = false; + const didHaveVideo = Boolean(localVideo); + const wasVideoMuted = this.videoMuted; + + return createLocalTracks({ + desktopSharingSources: options.desktopSharingSources, + devices: ['desktop'], + desktopSharingExtensionExternalInstallation: { + interval: 500, + checkAgain: () => { + return DSExternalInstallationInProgress; + }, + listener: (status, url) => { + switch(status) { + case "waitingForExtension": { + DSExternalInstallationInProgress = true; + externalInstallation = true; + const listener = () => { + // Wait a little bit more just to be sure that + // we won't miss the extension installation + setTimeout( + () => { + DSExternalInstallationInProgress = false; + }, 500); + APP.UI.removeListener( + UIEvents.EXTERNAL_INSTALLATION_CANCELED, + listener); + }; + APP.UI.addListener( + UIEvents.EXTERNAL_INSTALLATION_CANCELED, + listener); + APP.UI.showExtensionExternalInstallationDialog(url); + break; + } + case "extensionFound": { + if (externalInstallation) //close the dialog + $.prompt.close(); + break; + } + default: { + //Unknown status + } + } + } + } + }).then(([desktopStream])=> { + // Stores the "untoggle" handler which remembers whether was + // there any video before and whether was it muted. + this._untoggleScreenSharing + = this._turnScreenSharingOff + .bind(this, didHaveVideo, wasVideoMuted); + desktopStream.on( + TrackEvents.LOCAL_TRACK_STOPPED, + () => { + // If the stream was stopped during screen sharing + // session then we should switch back to video. + if (this.isSharingScreen) { + this._untoggleScreenSharing + && this._untoggleScreenSharing(); + } + } + ); + // close external installation dialog on success. + if (externalInstallation) { + $.prompt.close(); + } + return desktopStream; + }, error => { + DSExternalInstallationInProgress = false; + // close external installation dialog on success. + if (externalInstallation) { + $.prompt.close(); + } + throw error; + }); + }, + + /** + * Tries to switch to the screenshairng mode by disposing camera stream and + * replacing it with a desktop one. + * + * @param {Object} [options] - Screen sharing options that will be passed to + * createLocalTracks. + * + * @return {Promise} - A Promise resolved if the operation succeeds or + * rejected with some unknown type of error in case it fails. Promise will + * be rejected immediately if {@link videoSwitchInProgress} is true. + * + * @private + */ + _switchToScreenSharing(options = {}) { + if (this.videoSwitchInProgress) { + return Promise.reject('Switch in progress.'); + } + + this.videoSwitchInProgress = true; + return this._createDesktopTrack(options).then(stream => { + return this.useVideoStream(stream); + }).then(() => { + this.videoSwitchInProgress = false; + JitsiMeetJS.analytics.sendEvent('conference.sharingDesktop.start'); + logger.log('sharing local desktop'); + }).catch(error => { + this.videoSwitchInProgress = false; + // Pawel: With this call I'm trying to preserve the original + // behaviour although it is not clear why would we "untoggle" + // on failure. I suppose it was to restore video in case there + // was some problem during "this.useVideoStream(desktopStream)". + // It's important to note that the handler will not be available + // if we fail early on trying to get desktop media (which makes + // sense, because the camera video is still being used, so + // nothing to "untoggle"). + if (this._untoggleScreenSharing) { + this._untoggleScreenSharing(); + } + + // FIXME the code inside of _handleScreenSharingError is + // asynchronous, but does not return a Promise and is not part of + // the current Promise chain. + this._handleScreenSharingError(error); + }); + }, + + /** + * Handles {@link JitsiTrackError} returned by the lib-jitsi-meet when + * trying to create screensharing track. It will either do nothing if + * the dialog was canceled on user's request or display inline installation + * dialog and ask the user to install the extension, once the extension is + * installed it will switch the conference to screensharing. The last option + * is that an unrecoverable error dialog will be displayed. + * @param {JitsiTrackError} error - The error returned by + * {@link _createDesktopTrack} Promise. + * @private + */ + _handleScreenSharingError(error) { + if (error.name === TrackErrors.CHROME_EXTENSION_USER_CANCELED) { + return; + } + + logger.error('failed to share local desktop', error); + + if (error.name === TrackErrors.CHROME_EXTENSION_USER_GESTURE_REQUIRED) { + // If start with screen sharing the extension will fail to install + // (if not found), because the request has been triggered by the + // script. Show a dialog which asks user to click "install" and try + // again switching to the screen sharing. + APP.UI.showExtensionInlineInstallationDialog( + () => { + this.toggleScreenSharing(); + } + ); + + return; + } else if (error.name === TrackErrors.FIREFOX_EXTENSION_NEEDED) { + APP.UI.showExtensionRequiredDialog( + config.desktopSharingFirefoxExtensionURL + ); + + return; + } + + // Handling: + // TrackErrors.PERMISSION_DENIED + // TrackErrors.CHROME_EXTENSION_INSTALLATION_ERROR + // TrackErrors.GENERAL + // and any other + let dialogTxt; + let dialogTitleKey; + + if (error.name === TrackErrors.PERMISSION_DENIED) { + dialogTxt = APP.translation.generateTranslationHTML( + "dialog.screenSharingPermissionDeniedError"); + dialogTitleKey = "dialog.error"; + } else { + dialogTxt = APP.translation.generateTranslationHTML( + "dialog.failtoinstall"); + dialogTitleKey = "dialog.permissionDenied"; + } + + APP.UI.messageHandler.openDialog(dialogTitleKey, dialogTxt, false); + }, /** * Setup interaction between conference and UI. */ @@ -1619,17 +1740,6 @@ export default { APP.UI.updateDTMFSupport(isDTMFSupported); }); - APP.UI.addListener(UIEvents.EXTERNAL_INSTALLATION_CANCELED, () => { - // Wait a little bit more just to be sure that we won't miss the - // extension installation - setTimeout(() => DSExternalInstallationInProgress = false, 500); - }); - APP.UI.addListener(UIEvents.OPEN_EXTENSION_STORE, (url) => { - window.open( - url, "extension_store_window", - "resizable,scrollbars=yes,status=1"); - }); - APP.UI.addListener(UIEvents.AUDIO_MUTED, muteLocalAudio); APP.UI.addListener(UIEvents.VIDEO_MUTED, muted => { if (this.isAudioOnly() && !muted) { diff --git a/config.js b/config.js index 9d1c41367..92046b647 100644 --- a/config.js +++ b/config.js @@ -69,6 +69,7 @@ var config = { // eslint-disable-line no-unused-vars // page redirection when call is hangup disableSimulcast: false, // requireDisplayName: true, // Forces the participants that doesn't have display name to enter it when they enter the room. + startScreenSharing: false, // Will try to start with screensharing instead of camera // startAudioMuted: 10, // every participant after the Nth will start audio muted // startVideoMuted: 10, // every participant after the Nth will start video muted // defaultLanguage: "en", diff --git a/lang/main.json b/lang/main.json index 461854422..fd71fd126 100644 --- a/lang/main.json +++ b/lang/main.json @@ -332,6 +332,8 @@ "goToStore": "Go to the webstore", "externalInstallationTitle": "Extension required", "externalInstallationMsg": "You need to install our desktop sharing extension.", + "inlineInstallationMsg": "You need to install our desktop sharing extension.", + "inlineInstallExtension": "Install now", "muteParticipantTitle": "Mute this participant?", "muteParticipantBody": "You won't be able to unmute them, but they can unmute themselves at any time.", "muteParticipantButton": "Mute", diff --git a/modules/UI/UI.js b/modules/UI/UI.js index 723b500c0..af89d36a6 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -1158,15 +1158,34 @@ UI.showExtensionRequiredDialog = function (url) { * @param url {string} the url of the extension. */ UI.showExtensionExternalInstallationDialog = function (url) { + let openedWindow = null; + let submitFunction = function(e,v){ if (v) { e.preventDefault(); - eventEmitter.emit(UIEvents.OPEN_EXTENSION_STORE, url); + if (openedWindow === null || openedWindow.closed) { + openedWindow + = window.open( + url, + "extension_store_window", + "resizable,scrollbars=yes,status=1"); + } else { + openedWindow.focus(); + } } }; - let closeFunction = function () { - eventEmitter.emit(UIEvents.EXTERNAL_INSTALLATION_CANCELED); + let closeFunction = function (e, v) { + if (openedWindow) { + // Ideally we would close the popup, but this does not seem to work + // on Chrome. Leaving it uncommented in case it could work + // in some version. + openedWindow.close(); + openedWindow = null; + } + if (!v) { + eventEmitter.emit(UIEvents.EXTERNAL_INSTALLATION_CANCELED); + } }; messageHandler.openTwoButtonDialog({ @@ -1179,6 +1198,36 @@ UI.showExtensionExternalInstallationDialog = function (url) { }); }; +/** + * Shows a dialog which asks user to install the extension. This one is + * displayed after installation is triggered from the script, but fails because + * it must be initiated by user gesture. + * @param callback {function} function to be executed after user clicks + * the install button - it should make another attempt to install the extension. + */ +UI.showExtensionInlineInstallationDialog = function (callback) { + let submitFunction = function(e,v){ + if (v) { + callback(); + } + }; + + let closeFunction = function (e, v) { + if (!v) { + eventEmitter.emit(UIEvents.EXTERNAL_INSTALLATION_CANCELED); + } + }; + + messageHandler.openTwoButtonDialog({ + titleKey: 'dialog.externalInstallationTitle', + msgKey: 'dialog.inlineInstallationMsg', + leftButtonKey: 'dialog.inlineInstallExtension', + submitFunction, + loadedFunction: $.noop, + closeFunction + }); +}; + /** * Shows dialog with combined information about camera and microphone errors. diff --git a/service/UI/UIEvents.js b/service/UI/UIEvents.js index d4d445120..3f033d1aa 100644 --- a/service/UI/UIEvents.js +++ b/service/UI/UIEvents.js @@ -97,12 +97,6 @@ export default { // changed. RESOLUTION_CHANGED: "UI.resolution_changed", - /** - * Notifies that the button "Go to webstore" is pressed on the dialog for - * external extension installation. - */ - OPEN_EXTENSION_STORE: "UI.open_extension_store", - /** * Notifies that the button "Cancel" is pressed on the dialog for * external extension installation. From 117d3bb1103a9c5efdb78d392e9f091289847026 Mon Sep 17 00:00:00 2001 From: paweldomas Date: Thu, 29 Jun 2017 20:49:00 +0200 Subject: [PATCH 3/5] ref(conference.js): show screensharing error first If there will be microphone error it will cover any screensharing dialog, but it's still better than in the reverse order where the screensharing dialog will sometime be closing the microphone alert ($.prompt.close(); is called). --- conference.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/conference.js b/conference.js index b4fdebeef..f41cc5eb6 100644 --- a/conference.js +++ b/conference.js @@ -558,6 +558,14 @@ export default { .then(([tracks, con]) => { APP.store.dispatch( mediaPermissionPromptVisibilityChanged(false)); + // FIXME If there will be microphone error it will cover any + // screensharing dialog, but it's still better than in + // the reverse order where the screensharing dialog will + // sometime be closing the microphone alert ($.prompt.close(); + // is called). Need to figure out dialogs chaining to fix that. + if (screenSharingError) { + this._handleScreenSharingError(screenSharingError); + } if (audioAndVideoError || audioOnlyError) { if (audioOnlyError || videoOnlyError) { // If both requests for 'audio' + 'video' and 'audio' @@ -573,18 +581,6 @@ export default { } } - // FIXME If there was a screen sharing error or the extension - // needs to be installed it will appear on top of eventual - // "microphone error" dialog. That is not great, but currently - // it's pretty hard to chain dialogs since they don't return - // Promises. - if (screenSharingError) { - // FIXME if _handleScreenSharingError will be dealing with - // installing external extension it may close previously - // opened microphone dialog ($.prompt.close(); is called). - this._handleScreenSharingError(screenSharingError); - } - return [tracks, con]; }); }, From 3fbb022ffb9509ac471baa96a70e83e421f2c4d6 Mon Sep 17 00:00:00 2001 From: paweldomas Date: Tue, 18 Jul 2017 12:25:37 +0200 Subject: [PATCH 4/5] ref(conference): use options in init tracks --- conference.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/conference.js b/conference.js index f41cc5eb6..5eebd4766 100644 --- a/conference.js +++ b/conference.js @@ -493,11 +493,12 @@ export default { * show guidance overlay for users on how to give access to camera and/or * microphone, * @param {string} roomName - * @param {boolean} startScreenSharing - if true should start with - * screensharing instead of camera video. + * @param {object} options + * @param {boolean} options.startScreenSharing - if true should + * start with screensharing instead of camera video. * @returns {Promise.} */ - createInitialLocalTracksAndConnect(roomName, startScreenSharing) { + createInitialLocalTracksAndConnect(roomName, options = {}) { let audioAndVideoError, audioOnlyError, screenSharingError, @@ -514,7 +515,7 @@ export default { let tryCreateLocalTracks; // FIXME the logic about trying to go audio only on error is duplicated - if (startScreenSharing) { + if (options.startScreenSharing) { tryCreateLocalTracks = this._createDesktopTrack() .then(desktopStream => { return createLocalTracks({ devices: ['audio'] }, true) @@ -621,8 +622,9 @@ export default { ).then(() => { analytics.init(); return this.createInitialLocalTracksAndConnect( - options.roomName, - config.startScreenSharing); + options.roomName, { + startScreenSharing: config.startScreenSharing + }); }).then(([tracks, con]) => { tracks.forEach(track => { if((track.isAudioTrack() && initialAudioMutedState) From 8093043d394a4d141e860758b368a54b8ae3f87b Mon Sep 17 00:00:00 2001 From: paweldomas Date: Tue, 18 Jul 2017 12:28:27 +0200 Subject: [PATCH 5/5] style(conference.js): style fixes --- conference.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/conference.js b/conference.js index 5eebd4766..c90e688e5 100644 --- a/conference.js +++ b/conference.js @@ -562,7 +562,7 @@ export default { // FIXME If there will be microphone error it will cover any // screensharing dialog, but it's still better than in // the reverse order where the screensharing dialog will - // sometime be closing the microphone alert ($.prompt.close(); + // sometimes be closing the microphone alert ($.prompt.close(); // is called). Need to figure out dialogs chaining to fix that. if (screenSharingError) { this._handleScreenSharingError(screenSharingError); @@ -570,7 +570,7 @@ export default { if (audioAndVideoError || audioOnlyError) { if (audioOnlyError || videoOnlyError) { // If both requests for 'audio' + 'video' and 'audio' - // only failed, we assume that there is some problems + // only failed, we assume that there are some problems // with user's microphone and show corresponding dialog. APP.UI.showDeviceErrorDialog( audioOnlyError, videoOnlyError); @@ -1307,8 +1307,9 @@ export default { break; } case "extensionFound": { - if (externalInstallation) //close the dialog + if (externalInstallation) { //close the dialog $.prompt.close(); + } break; } default: { @@ -1317,7 +1318,7 @@ export default { } } } - }).then(([desktopStream])=> { + }).then(([desktopStream]) => { // Stores the "untoggle" handler which remembers whether was // there any video before and whether was it muted. this._untoggleScreenSharing