Skip to content

Commit b1b2804

Browse files
Rob--WRob--W
authored
fix: use getRoot request to support Firefox 77+ (#1886)
1 parent e0d4c3c commit b1b2804

File tree

3 files changed

+110
-26
lines changed

3 files changed

+110
-26
lines changed

‎src/firefox/remote.js

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,27 +86,50 @@ export class RemoteFirefox {
8686
});
8787
}
8888

89+
getAddonsActor(): Promise<string> {
90+
return new Promise((resolve, reject) => {
91+
// getRoot should work since Firefox 55 (bug 1352157).
92+
this.client.request('getRoot', (err, actors) => {
93+
if (!err) {
94+
if (actors.addonsActor) {
95+
resolve(actors.addonsActor);
96+
} else {
97+
reject(new RemoteTempInstallNotSupported(
98+
'This version of Firefox does not provide an add-ons actor for ' +
99+
'remote installation.'));
100+
}
101+
return;
102+
}
103+
log.debug(`Falling back to listTabs because getRoot failed: ${err}`);
104+
// Fallback to listTabs otherwise, Firefox 49 - 77 (bug 1618691).
105+
this.client.request('listTabs', (error, tabsResponse) => {
106+
if (error) {
107+
return reject(new WebExtError(
108+
`Remote Firefox: listTabs() error: ${error}`));
109+
}
110+
// addonsActor was added to listTabs in Firefox 49 (bug 1273183).
111+
if (!tabsResponse.addonsActor) {
112+
log.debug(
113+
'listTabs returned a falsey addonsActor: ' +
114+
`${tabsResponse.addonsActor}`);
115+
return reject(new RemoteTempInstallNotSupported(
116+
'This is an older version of Firefox that does not provide an ' +
117+
'add-ons actor for remote installation. Try Firefox 49 or ' +
118+
'higher.'));
119+
}
120+
resolve(tabsResponse.addonsActor);
121+
});
122+
});
123+
});
124+
}
125+
89126
installTemporaryAddon(
90127
addonPath: string
91128
): Promise<FirefoxRDPResponseAddon> {
92129
return new Promise((resolve, reject) => {
93-
this.client.request('listTabs', (error, tabsResponse) => {
94-
if (error) {
95-
return reject(new WebExtError(
96-
`Remote Firefox: listTabs() error: ${error}`));
97-
}
98-
if (!tabsResponse.addonsActor) {
99-
log.debug(
100-
'listTabs returned a falsey addonsActor: ' +
101-
`${tabsResponse.addonsActor}`);
102-
return reject(new RemoteTempInstallNotSupported(
103-
'This is an older version of Firefox that does not provide an ' +
104-
'add-ons actor for remote installation. Try Firefox 49 or ' +
105-
'higher.'));
106-
}
107-
130+
this.getAddonsActor().then((addonsActor) => {
108131
this.client.client.makeRequest({
109-
to: tabsResponse.addonsActor,
132+
to: addonsActor,
110133
type: 'installTemporaryAddon',
111134
addonPath,
112135
}, (installResponse) => {
@@ -120,7 +143,7 @@ export class RemoteFirefox {
120143
log.info(`Installed ${addonPath} as a temporary add-on`);
121144
resolve(installResponse);
122145
});
123-
});
146+
}).catch(reject);
124147
});
125148
}
126149

‎tests/functional/fake-firefox-binary.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
const net = require('net');
44

55
const REPLY_INITIAL = {from: 'root'};
6-
const REQUEST_LIST_TABS = {to: 'root', type: 'listTabs'};
7-
const REPLY_LIST_TABS = {from: 'root', addonsActor: 'fakeAddonsActor'};
6+
const REQUEST_ACTORS = {to: 'root', type: 'getRoot'};
7+
const REPLY_ACTORS = {from: 'root', addonsActor: 'fakeAddonsActor'};
88
const REQUEST_INSTALL_ADDON = {
99
to: 'fakeAddonsActor',
1010
type: 'installTemporaryAddon',
@@ -37,8 +37,8 @@ function getPortFromArgs() {
3737
}
3838
net.createServer(function(socket) {
3939
socket.on('data', function(data) {
40-
if (String(data) === toRDP(REQUEST_LIST_TABS)) {
41-
socket.write(toRDP(REPLY_LIST_TABS));
40+
if (String(data) === toRDP(REQUEST_ACTORS)) {
41+
socket.write(toRDP(REPLY_ACTORS));
4242
} else if (String(data) === toRDP(REQUEST_INSTALL_ADDON)) {
4343
socket.write(toRDP(REPLY_INSTALL_ADDON));
4444

‎tests/unit/test-firefox/test.remote.js

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,9 +225,9 @@ describe('firefox.remote', () => {
225225

226226
describe('installTemporaryAddon', () => {
227227

228-
it('throws listTabs errors', async () => {
228+
it('throws getRoot errors', async () => {
229229
const client = fakeFirefoxClient({
230-
// listTabs response:
230+
// listTabs and getRoot response:
231231
requestError: new Error('some listTabs error'),
232232
});
233233
const conn = makeInstance(client);
@@ -236,24 +236,33 @@ describe('firefox.remote', () => {
236236
.catch(onlyInstancesOf(WebExtError, (error) => {
237237
assert.match(error.message, /some listTabs error/);
238238
}));
239+
240+
// When getRoot fails, a fallback to listTabs is expected.
241+
sinon.assert.calledTwice(client.request);
242+
sinon.assert.calledWith(client.request, 'getRoot');
243+
sinon.assert.calledWith(client.request, 'listTabs');
239244
});
240245

241246
it('fails when there is no add-ons actor', async () => {
242247
const client = fakeFirefoxClient({
243-
// A listTabs response that does not contain addonsActor.
248+
// A getRoot and listTabs response that does not contain addonsActor.
244249
requestResult: {},
245250
});
246251
const conn = makeInstance(client);
247252
await conn.installTemporaryAddon('/path/to/addon')
248253
.then(makeSureItFails())
249254
.catch(onlyInstancesOf(RemoteTempInstallNotSupported, (error) => {
250-
assert.match(error.message, /does not provide an add-ons actor/);
255+
assert.match(
256+
error.message,
257+
/This version of Firefox does not provide an add-ons actor/);
251258
}));
259+
sinon.assert.calledOnce(client.request);
260+
sinon.assert.calledWith(client.request, 'getRoot');
252261
});
253262

254263
it('lets you install an add-on temporarily', async () => {
255264
const client = fakeFirefoxClient({
256-
// listTabs response:
265+
// getRoot response:
257266
requestResult: {
258267
addonsActor: 'addons1.actor.conn',
259268
},
@@ -265,6 +274,58 @@ describe('firefox.remote', () => {
265274
const conn = makeInstance(client);
266275
const response = await conn.installTemporaryAddon('/path/to/addon');
267276
assert.equal(response.addon.id, 'abc123@temporary-addon');
277+
278+
// When called without error, there should not be any fallback.
279+
sinon.assert.calledOnce(client.request);
280+
sinon.assert.calledWith(client.request, 'getRoot');
281+
});
282+
283+
it('falls back to listTabs when getRoot is unavailable', async () => {
284+
const client = fakeFirefoxClient({
285+
// installTemporaryAddon response:
286+
makeRequestResult: {
287+
addon: {id: 'abc123@temporary-addon'},
288+
},
289+
});
290+
client.request = sinon.stub();
291+
// Sample response from Firefox 49.
292+
client.request.withArgs('getRoot').callsArgWith(1, {
293+
error: 'unrecognizedPacketType',
294+
message: 'Actor root does not recognize the packet type getRoot',
295+
});
296+
client.request.withArgs('listTabs').callsArgWith(1, undefined, {
297+
addonsActor: 'addons1.actor.conn',
298+
});
299+
const conn = makeInstance(client);
300+
const response = await conn.installTemporaryAddon('/path/to/addon');
301+
assert.equal(response.addon.id, 'abc123@temporary-addon');
302+
303+
sinon.assert.calledTwice(client.request);
304+
sinon.assert.calledWith(client.request, 'getRoot');
305+
sinon.assert.calledWith(client.request, 'listTabs');
306+
});
307+
308+
it('fails when getRoot and listTabs both fail', async () => {
309+
const client = fakeFirefoxClient();
310+
client.request = sinon.stub();
311+
// Sample response from Firefox 48.
312+
client.request.withArgs('getRoot').callsArgWith(1, {
313+
error: 'unrecognizedPacketType',
314+
message: 'Actor root does not recognize the packet type getRoot',
315+
});
316+
client.request.withArgs('listTabs').callsArgWith(1, undefined, {});
317+
const conn = makeInstance(client);
318+
await conn.installTemporaryAddon('/path/to/addon')
319+
.then(makeSureItFails())
320+
.catch(onlyInstancesOf(RemoteTempInstallNotSupported, (error) => {
321+
assert.match(
322+
error.message,
323+
/does not provide an add-ons actor.*Try Firefox 49/);
324+
}));
325+
326+
sinon.assert.calledTwice(client.request);
327+
sinon.assert.calledWith(client.request, 'getRoot');
328+
sinon.assert.calledWith(client.request, 'listTabs');
268329
});
269330

270331
it('throws install errors', async () => {

0 commit comments

Comments
 (0)