MM-51956 - Fix for: Calls popout generating duplicate notifications (#2713)
* set userAgent in the widget popout * tests * set userAgent on widget, for consistency * added link to ticket for the proper fix
This commit is contained in:

committed by
GitHub

parent
925b2fcc32
commit
3b8bddd5e7
@@ -66,6 +66,7 @@ jest.mock('../utils', () => ({
|
|||||||
openScreensharePermissionsSettingsMacOS: jest.fn(),
|
openScreensharePermissionsSettingsMacOS: jest.fn(),
|
||||||
resetScreensharePermissionsMacOS: jest.fn(),
|
resetScreensharePermissionsMacOS: jest.fn(),
|
||||||
getLocalPreload: jest.fn(),
|
getLocalPreload: jest.fn(),
|
||||||
|
composeUserAgent: jest.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
describe('main/windows/callsWidgetWindow', () => {
|
describe('main/windows/callsWidgetWindow', () => {
|
||||||
@@ -401,6 +402,7 @@ describe('main/windows/callsWidgetWindow', () => {
|
|||||||
it('onPopOutCreate - should attach correct listeners and should prevent redirects', () => {
|
it('onPopOutCreate - should attach correct listeners and should prevent redirects', () => {
|
||||||
let redirectListener;
|
let redirectListener;
|
||||||
let closedListener;
|
let closedListener;
|
||||||
|
let frameFinishedLoadListener;
|
||||||
const popOut = {
|
const popOut = {
|
||||||
on: (event, listener) => {
|
on: (event, listener) => {
|
||||||
closedListener = listener;
|
closedListener = listener;
|
||||||
@@ -409,8 +411,13 @@ describe('main/windows/callsWidgetWindow', () => {
|
|||||||
on: (event, listener) => {
|
on: (event, listener) => {
|
||||||
redirectListener = listener;
|
redirectListener = listener;
|
||||||
},
|
},
|
||||||
|
once: (event, listener) => {
|
||||||
|
frameFinishedLoadListener = listener;
|
||||||
|
},
|
||||||
id: 'webContentsId',
|
id: 'webContentsId',
|
||||||
|
getURL: () => ('http://myurl.com'),
|
||||||
},
|
},
|
||||||
|
loadURL: jest.fn(),
|
||||||
};
|
};
|
||||||
|
|
||||||
const callsWidgetWindow = new CallsWidgetWindow();
|
const callsWidgetWindow = new CallsWidgetWindow();
|
||||||
@@ -418,11 +425,15 @@ describe('main/windows/callsWidgetWindow', () => {
|
|||||||
expect(callsWidgetWindow.popOut).toBe(popOut);
|
expect(callsWidgetWindow.popOut).toBe(popOut);
|
||||||
expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents);
|
expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents);
|
||||||
expect(redirectListener).toBeDefined();
|
expect(redirectListener).toBeDefined();
|
||||||
|
expect(frameFinishedLoadListener).toBeDefined();
|
||||||
|
|
||||||
const event = {preventDefault: jest.fn()};
|
const event = {preventDefault: jest.fn()};
|
||||||
redirectListener(event);
|
redirectListener(event);
|
||||||
expect(event.preventDefault).toHaveBeenCalled();
|
expect(event.preventDefault).toHaveBeenCalled();
|
||||||
|
|
||||||
|
frameFinishedLoadListener();
|
||||||
|
expect(callsWidgetWindow.popOut.loadURL).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
closedListener();
|
closedListener();
|
||||||
expect(callsWidgetWindow.popOut).not.toBeDefined();
|
expect(callsWidgetWindow.popOut).not.toBeDefined();
|
||||||
});
|
});
|
||||||
|
@@ -16,7 +16,12 @@ import {
|
|||||||
|
|
||||||
import {MattermostBrowserView} from 'main/views/MattermostBrowserView';
|
import {MattermostBrowserView} from 'main/views/MattermostBrowserView';
|
||||||
|
|
||||||
import {getLocalPreload, openScreensharePermissionsSettingsMacOS, resetScreensharePermissionsMacOS} from 'main/utils';
|
import {
|
||||||
|
composeUserAgent,
|
||||||
|
getLocalPreload,
|
||||||
|
openScreensharePermissionsSettingsMacOS,
|
||||||
|
resetScreensharePermissionsMacOS,
|
||||||
|
} from 'main/utils';
|
||||||
|
|
||||||
import {Logger} from 'common/log';
|
import {Logger} from 'common/log';
|
||||||
import {CALLS_PLUGIN_ID, MINIMUM_CALLS_WIDGET_HEIGHT, MINIMUM_CALLS_WIDGET_WIDTH} from 'common/utils/constants';
|
import {CALLS_PLUGIN_ID, MINIMUM_CALLS_WIDGET_HEIGHT, MINIMUM_CALLS_WIDGET_WIDTH} from 'common/utils/constants';
|
||||||
@@ -154,7 +159,9 @@ export class CallsWidgetWindow {
|
|||||||
if (!widgetURL) {
|
if (!widgetURL) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
this.win?.loadURL(widgetURL).catch((reason) => {
|
this.win?.loadURL(widgetURL, {
|
||||||
|
userAgent: composeUserAgent(),
|
||||||
|
}).catch((reason) => {
|
||||||
log.error(`failed to load: ${reason}`);
|
log.error(`failed to load: ${reason}`);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -262,7 +269,7 @@ export class CallsWidgetWindow {
|
|||||||
this.setBounds(initialBounds);
|
this.setBounds(initialBounds);
|
||||||
}
|
}
|
||||||
|
|
||||||
private onPopOutOpen = ({url}: {url: string}) => {
|
private onPopOutOpen = ({url}: { url: string }) => {
|
||||||
if (!(this.mainView && this.options)) {
|
if (!(this.mainView && this.options)) {
|
||||||
return {action: 'deny' as const};
|
return {action: 'deny' as const};
|
||||||
}
|
}
|
||||||
@@ -300,6 +307,24 @@ export class CallsWidgetWindow {
|
|||||||
this.popOut.on('closed', () => {
|
this.popOut.on('closed', () => {
|
||||||
delete this.popOut;
|
delete this.popOut;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Set the userAgent so that the widget's popout is considered a desktop window in the webapp code.
|
||||||
|
// 'did-frame-finish-load' is the earliest moment that allows us to call loadURL without throwing an error.
|
||||||
|
// https://mattermost.atlassian.net/browse/MM-52756 is the proper fix for this.
|
||||||
|
this.popOut.webContents.once('did-frame-finish-load', async () => {
|
||||||
|
const url = this.popOut?.webContents.getURL() || '';
|
||||||
|
if (!url) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
await this.popOut?.loadURL(url, {
|
||||||
|
userAgent: composeUserAgent(),
|
||||||
|
});
|
||||||
|
} catch (e) {
|
||||||
|
log.error('did-frame-finish-load, failed to reload with correct userAgent', e);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/************************
|
/************************
|
||||||
|
Reference in New Issue
Block a user