From ae06a6ce414e43c2127530724d0ba8d3c8f7db0e Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Wed, 12 Apr 2017 14:06:56 -0500 Subject: [PATCH] Fix Recording regression caused by 'React Toolbar' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saúl Ibarra Corretgé reported that Recording shows an error dialog stating "There was an error connecting to your camera". Hristo Terezov and Yana Stamcheva traced that the problem originates in https://github.com/jitsi/jitsi-meet/commit/da4425b5c0eb8ec06dfa9d350c15a27460f3dec2 and, more specifically, is caused by a different order of execution due to the move of the invocation of the function Recording.init. The solution is to bring back the execution location of Recording.init. --- modules/UI/UI.js | 4 + modules/UI/recording/Recording.js | 256 +++++++++++------------ modules/UI/util/UIUtil.js | 10 +- react/features/toolbox/actionTypes.js | 30 ++- react/features/toolbox/actions.native.js | 19 +- react/features/toolbox/actions.web.js | 18 +- react/features/toolbox/reducer.js | 29 ++- 7 files changed, 208 insertions(+), 158 deletions(-) diff --git a/modules/UI/UI.js b/modules/UI/UI.js index 483b5d4e0..b94bf8523 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -322,6 +322,10 @@ UI.start = function () { $("#videoconference_page").mousemove(debouncedShowToolbar); + // Initialise the recording module. + if (config.enableRecording) { + Recording.init(eventEmitter, config.recordingType); + } // Initialize side panels SidePanels.init(eventEmitter); } else { diff --git a/modules/UI/recording/Recording.js b/modules/UI/recording/Recording.js index 952810d4d..5046b9cfd 100644 --- a/modules/UI/recording/Recording.js +++ b/modules/UI/recording/Recording.js @@ -21,7 +21,7 @@ import UIUtil from '../util/UIUtil'; import VideoLayout from '../videolayout/VideoLayout'; import Feedback from '../feedback/Feedback.js'; -import { hideToolbox } from '../../../react/features/toolbox'; +import { setToolboxEnabled } from '../../../react/features/toolbox'; /** * The dialog for user input. @@ -35,8 +35,10 @@ let dialog = null; * @private */ function _isRecordingButtonEnabled() { - return interfaceConfig.TOOLBAR_BUTTONS.indexOf("recording") !== -1 - && config.enableRecording && APP.conference.isRecordingSupported(); + return ( + interfaceConfig.TOOLBAR_BUTTONS.indexOf("recording") !== -1 + && config.enableRecording + && APP.conference.isRecordingSupported()); } /** @@ -129,7 +131,7 @@ function _requestLiveStreamId() { * Request recording token from the user. * @returns {Promise} */ -function _requestRecordingToken () { +function _requestRecordingToken() { let titleKey = "dialog.recordingToken"; let messageString = ( ` { dialog = APP.UI.messageHandler.openTwoButtonDialog({ titleKey: title, msgKey: message, leftButtonKey: buttonKey, - submitFunction: function(e,v) { - if (v) { - resolve(); - } else { - reject(); - } - }, - closeFunction: function () { + submitFunction: (e, v) => (v ? resolve : reject)(), + closeFunction: () => { dialog = null; } }); @@ -250,35 +246,12 @@ var Recording = { /** * Initializes the recording UI. */ - init (emitter, recordingType) { - this.eventEmitter = emitter; + init(eventEmitter, recordingType) { + this.eventEmitter = eventEmitter; + this.recordingType = recordingType; this.updateRecordingState(APP.conference.getRecordingState()); - this.initRecordingButton(recordingType); - - // If I am a recorder then I publish my recorder custom role to notify - // everyone. - if (config.iAmRecorder) { - VideoLayout.enableDeviceAvailabilityIcons( - APP.conference.getMyUserId(), false); - VideoLayout.setLocalVideoVisible(false); - Feedback.enableFeedback(false); - APP.store.dispatch(hideToolbox()); - APP.UI.messageHandler.enableNotifications(false); - APP.UI.messageHandler.enablePopups(false); - } - }, - - /** - * Initialise the recording button. - */ - initRecordingButton(recordingType) { - let selector = $('#toolbar_button_record'); - - let button = selector.get(0); - UIUtil.setTooltip(button, 'liveStreaming.buttonTooltip', 'right'); - if (recordingType === 'jibri') { this.baseClass = "fa fa-play-circle"; this.recordingTitle = "dialog.liveStreaming"; @@ -304,101 +277,44 @@ var Recording = { this.recordingBusy = "liveStreaming.busy"; } + // XXX Due to the React-ification of Toolbox, the HTMLElement with id + // toolbar_button_record may not exist yet. + $(document).on( + 'click', + '#toolbar_button_record', + ev => this._onToolbarButtonClick(ev)); + + // If I am a recorder then I publish my recorder custom role to notify + // everyone. + if (config.iAmRecorder) { + VideoLayout.enableDeviceAvailabilityIcons( + APP.conference.getMyUserId(), false); + VideoLayout.setLocalVideoVisible(false); + Feedback.enableFeedback(false); + APP.store.dispatch(setToolboxEnabled(false)); + APP.UI.messageHandler.enableNotifications(false); + APP.UI.messageHandler.enablePopups(false); + } + }, + + /** + * Initialise the recording button. + */ + initRecordingButton() { + const selector = $('#toolbar_button_record'); + + UIUtil.setTooltip(selector, 'liveStreaming.buttonTooltip', 'right'); + selector.addClass(this.baseClass); selector.attr("data-i18n", "[content]" + this.recordingButtonTooltip); APP.translation.translateElement(selector); - - var self = this; - selector.click(function () { - if (dialog) - return; - JitsiMeetJS.analytics.sendEvent('recording.clicked'); - switch (self.currentState) { - case Status.ON: - case Status.RETRYING: - case Status.PENDING: { - _showStopRecordingPrompt(recordingType).then( - () => { - self.eventEmitter.emit(UIEvents.RECORDING_TOGGLED); - JitsiMeetJS.analytics.sendEvent( - 'recording.stopped'); - }, - () => {}); - break; - } - case Status.AVAILABLE: - case Status.OFF: { - if (recordingType === 'jibri') - _requestLiveStreamId().then((streamId) => { - self.eventEmitter.emit( UIEvents.RECORDING_TOGGLED, - {streamId: streamId}); - JitsiMeetJS.analytics.sendEvent( - 'recording.started'); - }).catch( - reason => { - if (reason !== APP.UI.messageHandler.CANCEL) - logger.error(reason); - else - JitsiMeetJS.analytics.sendEvent( - 'recording.canceled'); - } - ); - else { - if (self.predefinedToken) { - self.eventEmitter.emit( UIEvents.RECORDING_TOGGLED, - {token: self.predefinedToken}); - JitsiMeetJS.analytics.sendEvent( - 'recording.started'); - return; - } - - _requestRecordingToken().then((token) => { - self.eventEmitter.emit( UIEvents.RECORDING_TOGGLED, - {token: token}); - JitsiMeetJS.analytics.sendEvent( - 'recording.started'); - }).catch( - reason => { - if (reason !== APP.UI.messageHandler.CANCEL) - logger.error(reason); - else - JitsiMeetJS.analytics.sendEvent( - 'recording.canceled'); - } - ); - } - break; - } - case Status.BUSY: { - dialog = APP.UI.messageHandler.openMessageDialog( - self.recordingTitle, - self.recordingBusy, - null, - function () { - dialog = null; - } - ); - break; - } - default: { - dialog = APP.UI.messageHandler.openMessageDialog( - self.recordingTitle, - self.recordingUnavailable, - null, - function () { - dialog = null; - } - ); - } - } - }); }, /** * Shows or hides the 'recording' button. * @param show {true} to show the recording button, {false} to hide it */ - showRecordingButton (show) { + showRecordingButton(show) { let shouldShow = show && _isRecordingButtonEnabled(); let id = 'toolbar_button_record'; @@ -425,7 +341,7 @@ var Recording = { * Sets the state of the recording button. * @param recordingState gives us the current recording state */ - updateRecordingUI (recordingState) { + updateRecordingUI(recordingState) { let oldState = this.currentState; this.currentState = recordingState; @@ -491,7 +407,7 @@ var Recording = { }, // checks whether recording is enabled and whether we have params // to start automatically recording - checkAutoRecord () { + checkAutoRecord() { if (_isRecordingButtonEnabled && config.autoRecord) { this.predefinedToken = UIUtil.escapeHtml(config.autoRecordToken); this.eventEmitter.emit(UIEvents.RECORDING_TOGGLED, @@ -514,6 +430,90 @@ var Recording = { APP.translation.translateElement(labelSelector); }, + /** + * Handles {@code click} on {@code toolbar_button_record}. + * + * @returns {void} + */ + _onToolbarButtonClick() { + if (dialog) { + return; + } + + JitsiMeetJS.analytics.sendEvent('recording.clicked'); + switch (this.currentState) { + case Status.ON: + case Status.RETRYING: + case Status.PENDING: { + _showStopRecordingPrompt(this.recordingType).then( + () => { + this.eventEmitter.emit(UIEvents.RECORDING_TOGGLED); + JitsiMeetJS.analytics.sendEvent('recording.stopped'); + }, + () => {}); + break; + } + case Status.AVAILABLE: + case Status.OFF: { + if (this.recordingType === 'jibri') + _requestLiveStreamId().then(streamId => { + this.eventEmitter.emit( + UIEvents.RECORDING_TOGGLED, + { streamId }); + JitsiMeetJS.analytics.sendEvent('recording.started'); + }).catch(reason => { + if (reason !== APP.UI.messageHandler.CANCEL) + logger.error(reason); + else + JitsiMeetJS.analytics.sendEvent('recording.canceled'); + }); + else { + if (this.predefinedToken) { + this.eventEmitter.emit( + UIEvents.RECORDING_TOGGLED, + { token: this.predefinedToken }); + JitsiMeetJS.analytics.sendEvent('recording.started'); + return; + } + + _requestRecordingToken().then((token) => { + this.eventEmitter.emit( + UIEvents.RECORDING_TOGGLED, + { token }); + JitsiMeetJS.analytics.sendEvent('recording.started'); + }).catch(reason => { + if (reason !== APP.UI.messageHandler.CANCEL) + logger.error(reason); + else + JitsiMeetJS.analytics.sendEvent('recording.canceled'); + }); + } + break; + } + case Status.BUSY: { + dialog = APP.UI.messageHandler.openMessageDialog( + this.recordingTitle, + this.recordingBusy, + null, + () => { + dialog = null; + } + ); + break; + } + default: { + dialog = APP.UI.messageHandler.openMessageDialog( + this.recordingTitle, + this.recordingUnavailable, + null, + () => { + dialog = null; + } + ); + } + } + }, + /** * Sets the toggled state of the recording toolbar button. * diff --git a/modules/UI/util/UIUtil.js b/modules/UI/util/UIUtil.js index 4bf30afee..dc1237979 100644 --- a/modules/UI/util/UIUtil.js +++ b/modules/UI/util/UIUtil.js @@ -157,11 +157,13 @@ const IndicatorFontSizes = { * @param position the position of the tooltip in relation to the element */ setTooltip(element, key, position) { - if (element !== null) { - element.setAttribute('data-tooltip', TOOLTIP_POSITIONS[position]); - element.setAttribute('data-i18n', '[content]' + key); + if (element) { + const selector = element.jquery ? element : $(element); - APP.translation.translateElement($(element)); + selector.attr('data-tooltip', TOOLTIP_POSITIONS[position]); + selector.attr('data-i18n', `[content]${key}`); + + APP.translation.translateElement(selector); } }, diff --git a/react/features/toolbox/actionTypes.js b/react/features/toolbox/actionTypes.js index 9586654ae..aaf091d30 100644 --- a/react/features/toolbox/actionTypes.js +++ b/react/features/toolbox/actionTypes.js @@ -21,16 +21,6 @@ export const CLEAR_TOOLBOX_TIMEOUT = Symbol('CLEAR_TOOLBOX_TIMEOUT'); export const SET_DEFAULT_TOOLBOX_BUTTONS = Symbol('SET_DEFAULT_TOOLBOX_BUTTONS'); -/** - * The type of the action which sets the permanent visibility of the Toolbox. - * - * { - * type: SET_TOOLBOX_ALWAYS_VISIBLE, - * alwaysVisible: boolean - * } - */ -export const SET_TOOLBOX_ALWAYS_VISIBLE = Symbol('SET_TOOLBOX_ALWAYS_VISIBLE'); - /** * The type of the action which sets the conference subject. * @@ -73,6 +63,26 @@ export const SET_TOOLBAR_BUTTON = Symbol('SET_TOOLBAR_BUTTON'); */ export const SET_TOOLBAR_HOVERED = Symbol('SET_TOOLBAR_HOVERED'); +/** + * The type of the action which sets the permanent visibility of the Toolbox. + * + * { + * type: SET_TOOLBOX_ALWAYS_VISIBLE, + * alwaysVisible: boolean + * } + */ +export const SET_TOOLBOX_ALWAYS_VISIBLE = Symbol('SET_TOOLBOX_ALWAYS_VISIBLE'); + +/** + * The type of the (redux) action which enables/disables the Toolbox. + * + * { + * type: SET_TOOLBOX_ENABLED, + * enabled: boolean + * } + */ +export const SET_TOOLBOX_ENABLED = Symbol('SET_TOOLBOX_ENABLED'); + /** * The type of the action which sets a new Toolbox visibility timeout and its * delay. diff --git a/react/features/toolbox/actions.native.js b/react/features/toolbox/actions.native.js index aaef1f604..7e3b3e2d1 100644 --- a/react/features/toolbox/actions.native.js +++ b/react/features/toolbox/actions.native.js @@ -5,11 +5,12 @@ import type { Dispatch } from 'redux-thunk'; import { CLEAR_TOOLBOX_TIMEOUT, SET_DEFAULT_TOOLBOX_BUTTONS, - SET_TOOLBOX_ALWAYS_VISIBLE, SET_SUBJECT, SET_SUBJECT_SLIDE_IN, SET_TOOLBAR_BUTTON, SET_TOOLBAR_HOVERED, + SET_TOOLBOX_ALWAYS_VISIBLE, + SET_TOOLBOX_ENABLED, SET_TOOLBOX_TIMEOUT, SET_TOOLBOX_TIMEOUT_MS, SET_TOOLBOX_VISIBLE @@ -168,6 +169,22 @@ export function setToolboxAlwaysVisible(alwaysVisible: boolean): Object { /* eslint-disable flowtype/space-before-type-colon */ +/** + * Enables/disables the toolbox. + * + * @param {boolean} enabled - True to enable the toolbox or false to disable it. + * @returns {{ + * type: SET_TOOLBOX_ENABLED, + * enabled: boolean + * }} + */ +export function setToolboxEnabled(enabled: boolean): Object { + return { + type: SET_TOOLBOX_ENABLED, + enabled + }; +} + /** * Dispatches an action which sets new timeout and clears the previous one. * diff --git a/react/features/toolbox/actions.web.js b/react/features/toolbox/actions.web.js index 84a375b87..39d284eb3 100644 --- a/react/features/toolbox/actions.web.js +++ b/react/features/toolbox/actions.web.js @@ -3,8 +3,8 @@ import Recording from '../../../modules/UI/recording/Recording'; import SideContainerToggler from '../../../modules/UI/side_pannels/SideContainerToggler'; -import UIEvents from '../../../service/UI/UIEvents'; import UIUtil from '../../../modules/UI/util/UIUtil'; +import UIEvents from '../../../service/UI/UIEvents'; import { clearToolboxTimeout, @@ -171,14 +171,11 @@ export function showDialPadButton(show: boolean): Function { */ export function showRecordingButton(): Function { return (dispatch: Dispatch<*>) => { - const eventEmitter = APP.UI.eventEmitter; - const buttonName = 'recording'; - - dispatch(setToolbarButton(buttonName, { + dispatch(setToolbarButton('recording', { hidden: false })); - Recording.init(eventEmitter, config.recordingType); + Recording.initRecordingButton(); }; } @@ -234,9 +231,14 @@ export function showSIPCallButton(show: boolean): Function { export function showToolbox(timeout: number = 0): Object { return (dispatch: Dispatch<*>, getState: Function) => { const state = getState(); - const { alwaysVisible, timeoutMS, visible } = state['features/toolbox']; + const { + alwaysVisible, + enabled, + timeoutMS, + visible + } = state['features/toolbox']; - if (!visible) { + if (enabled && !visible) { dispatch(setToolboxVisible(true)); dispatch(setSubjectSlideIn(true)); diff --git a/react/features/toolbox/reducer.js b/react/features/toolbox/reducer.js index 2f3b756bd..7c8c5b9ec 100644 --- a/react/features/toolbox/reducer.js +++ b/react/features/toolbox/reducer.js @@ -5,11 +5,12 @@ import { ReducerRegistry } from '../base/redux'; import { CLEAR_TOOLBOX_TIMEOUT, SET_DEFAULT_TOOLBOX_BUTTONS, - SET_TOOLBOX_ALWAYS_VISIBLE, SET_SUBJECT, SET_SUBJECT_SLIDE_IN, SET_TOOLBAR_BUTTON, SET_TOOLBAR_HOVERED, + SET_TOOLBOX_ALWAYS_VISIBLE, + SET_TOOLBOX_ENABLED, SET_TOOLBOX_TIMEOUT, SET_TOOLBOX_TIMEOUT_MS, SET_TOOLBOX_VISIBLE @@ -51,6 +52,14 @@ function _getInitialState() { */ alwaysVisible: false, + /** + * The indicator which determines whether the Toolbox is enabled. For + * example, modules/UI/recording/Recording.js disables the Toolbox. + * + * @type {boolean} + */ + enabled: true, + /** * The indicator which determines whether a Toolbar in the Toolbox is * hovered. @@ -132,12 +141,6 @@ ReducerRegistry.register( }; } - case SET_TOOLBOX_ALWAYS_VISIBLE: - return { - ...state, - alwaysVisible: action.alwaysVisible - }; - case SET_SUBJECT: return { ...state, @@ -159,6 +162,18 @@ ReducerRegistry.register( hovered: action.hovered }; + case SET_TOOLBOX_ALWAYS_VISIBLE: + return { + ...state, + alwaysVisible: action.alwaysVisible + }; + + case SET_TOOLBOX_ENABLED: + return { + ...state, + enabled: action.enabled + }; + case SET_TOOLBOX_TIMEOUT: return { ...state,