From 3733e4231606d7ea090a122f36a90662610205c7 Mon Sep 17 00:00:00 2001 From: stoically Date: Fri, 23 Feb 2018 19:09:39 +0100 Subject: [PATCH] More extensive MAC tests with preferences-combinations Part of #29 --- package.json | 2 +- src/background/mac.js | 24 +++-- src/background/mouseclick.js | 11 ++- src/background/request.js | 108 ++++++++++++---------- test/background.mac.test.js | 170 +++++++++++++++++++++++++++++++---- test/helper.js | 24 +++-- 6 files changed, 255 insertions(+), 84 deletions(-) diff --git a/package.json b/package.json index 7a84e986..c4288fc1 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "eslint": "^4.11.0", "mocha": "^4.0.1", "require-reload": "^0.2.2", - "sinon": "^4.1.2", + "sinon": "^4.4.0", "sinon-chai": "^2.14.0", "web-ext": "^2.2.2", "webpack": "^3.10.0" diff --git a/src/background/mac.js b/src/background/mac.js index d53c4c31..cf6d0f40 100644 --- a/src/background/mac.js +++ b/src/background/mac.js @@ -25,10 +25,10 @@ class MultiAccountContainers { targetContainer: queryParams[1][1], currentContainer: queryParams[2] ? queryParams[2][1] : false }; - debug('[handleConfirmPage] parsed url', parsedURL, queryParams, confirmPage); + debug('[handleConfirmPage] parsed url', queryParams, confirmPage); if (this.waitingForConfirmPage[confirmPage.targetContainer]) { debug('[handleConfirmPage] we are already waiting for this confirm page, maybe reopen', confirmPage.targetContainer); - this._maybeReopenConfirmPage(this.waitingForConfirmPage[confirmPage.targetContainer]); + this._maybeReopenConfirmPage(this.waitingForConfirmPage[confirmPage.targetContainer], confirmPage); } else { debug('[handleConfirmPage] we remember that we saw this confirm page, maybe it needs to be reopened', confirmPage.targetContainer); this.confirmPage[confirmPage.targetContainer] = confirmPage; @@ -47,7 +47,7 @@ class MultiAccountContainers { debug('[maybeReopenConfirmPage] we saw a mac confirm page for the target container already', targetContainer); this._maybeReopenConfirmPage({targetContainer, request, tab, deletesHistoryContainer}); } else { - debug('[maybeReopenConfirmPage] we didnt saw a mac confirm page yet, waiting', targetContainer); + debug('[maybeReopenConfirmPage] we didnt saw a mac confirm page yet, waiting', targetContainer, tab); this.waitingForConfirmPage[targetContainer] = {targetContainer, request, tab, deletesHistoryContainer}; delay(2000).then(() => { debug('[maybeReopenConfirmPage] cleaning up', targetContainer); @@ -56,14 +56,22 @@ class MultiAccountContainers { } } - _maybeReopenConfirmPage({targetContainer, request, tab, deletesHistoryContainer}) { + _maybeReopenConfirmPage({targetContainer, request, tab, deletesHistoryContainer}, confirmPage) { debug('[_maybeReopenConfirmPage]', targetContainer, request, tab, deletesHistoryContainer); if (this.waitingForConfirmPage[targetContainer]) { delete this.waitingForConfirmPage[targetContainer]; } - const currentContainer = this.confirmPage[targetContainer].currentContainer; + if (!confirmPage) { + confirmPage = this.confirmPage[targetContainer]; + } + if (!confirmPage) { + debug('[_maybeReopenConfirmPage] something went wrong, aborting'); + return; + } + const currentContainer = confirmPage.currentContainer; if (currentContainer) { - if (this.storage.local.tempContainers[currentContainer].clean) { + if (this.storage.local.tempContainers[currentContainer] && + this.storage.local.tempContainers[currentContainer].clean) { debug('[_maybeReopenConfirmPage] the currentContainer mac confirm wants to open is a clean tmp container, we do nothing'); return; } else { @@ -74,9 +82,9 @@ class MultiAccountContainers { } this.container.reloadTabInTempContainer( - this.confirmPage[targetContainer].tab, + confirmPage.tab ? confirmPage.tab : undefined, request.url, - this.confirmPage[targetContainer].tab.active, + confirmPage.tab ? confirmPage.tab.active : undefined, deletesHistoryContainer, request ); diff --git a/src/background/mouseclick.js b/src/background/mouseclick.js index 3209179a..91dc741c 100644 --- a/src/background/mouseclick.js +++ b/src/background/mouseclick.js @@ -4,6 +4,7 @@ const { debug } = require('./log'); class MouseClick { constructor() { this.linksClicked = {}; + this.unhandledLinksClicked = {}; } @@ -37,9 +38,17 @@ class MouseClick { clickType = 'ctrlleft'; } if (!this.checkClick(clickType, message, sender)) { + this.unhandledLinksClicked[url] = {}; + setTimeout(() => { + debug('[linkClicked] cleaning up unhandledLinksClicked', url); + delete this.unhandledLinksClicked[url]; + }, 1000); return; } } + if (this.unhandledLinksClicked[url]) { + delete this.unhandledLinksClicked[url]; + } const tab = sender.tab; if (!this.linksClicked[url]) { @@ -56,7 +65,7 @@ class MouseClick { this.linksClicked[url].count++; setTimeout(() => { - debug('[runtimeOnMessage] cleaning up', url); + debug('[linkClicked] cleaning up linksClicked', url); delete this.linksClicked[url]; delete this.container.urlCreatedContainer[url]; }, 1000); diff --git a/src/background/request.js b/src/background/request.js index 9513f8b9..499f379e 100644 --- a/src/background/request.js +++ b/src/background/request.js @@ -50,6 +50,22 @@ class Request { async webRequestOnBeforeRequest(request) { + const returnVal = await this._webRequestOnBeforeRequest(request); + if (returnVal && returnVal.cancel) { + this.cancelRequest(request); + } else { + const cookieStoreId = this.storage.local.tabContainerMap[request.tabId]; + if (cookieStoreId && this.storage.local.tempContainers[cookieStoreId] && + this.storage.local.tempContainers[cookieStoreId].clean) { + debug('[webRequestOnBeforeRequest] marking tmp container as not clean anymore', cookieStoreId, request); + this.storage.local.tempContainers[cookieStoreId].clean = false; + } + } + return returnVal; + } + + + async _webRequestOnBeforeRequest(request) { debug('[browser.webRequest.onBeforeRequest] incoming request', request); if (request.tabId === -1) { debug('[browser.webRequest.onBeforeRequest] onBeforeRequest request doesnt belong to a tab, why are you main_frame?', request); @@ -77,27 +93,27 @@ class Request { return; } - let tab = { - id: request.tabId, - cookieStoreId: 'firefox-default' - }; + let tab; if (macAssignment) { if (macAssignment.neverAsk) { debug('[webRequestOnBeforeRequest] mac neverask assigned, we do nothing', macAssignment); return; } else { debug('[webRequestOnBeforeRequest] mac assigned', macAssignment); + // we just assume that we cant get the tab informations anymore + // so we dont even try } - } - try { - tab = await browser.tabs.get(request.tabId); - debug('[webRequestOnBeforeRequest] onbeforeRequest requested tab information', tab); - if (macAssignment && tab.cookieStoreId === macAssignment.cookieStoreId) { - debug('[webRequestOnBeforeRequest] the request url is mac assigned to this container, we do nothing'); - return; + } else { + try { + tab = await browser.tabs.get(request.tabId); + debug('[webRequestOnBeforeRequest] onbeforeRequest requested tab information', tab); + if (macAssignment && tab.cookieStoreId === macAssignment.cookieStoreId) { + debug('[webRequestOnBeforeRequest] the request url is mac assigned to this container, we do nothing'); + return; + } + } catch (error) { + debug('[webRequestOnBeforeRequest] onbeforeRequest retrieving tab information failed, mac was probably faster', error); } - } catch (error) { - debug('[webRequestOnBeforeRequest] onbeforeRequest retrieving tab information failed, mac was probably faster', error); } this.container.maybeAddHistory(tab, request.url); @@ -110,9 +126,12 @@ class Request { !this.mouseclick.linksClicked[request.url]) { debug('[webRequestOnBeforeRequest] automatic mode disabled and no link clicked', request); return; + } else if (this.mouseclick.unhandledLinksClicked[request.url]) { + debug('[webRequestOnBeforeRequest] we saw an unhandled click for that url', request); + return; } - if (alwaysOpenIn && !this.mouseclick.linksClicked[request.url] && tab.openerTabId) { + if (alwaysOpenIn && !this.mouseclick.linksClicked[request.url] && tab && tab.openerTabId) { // TODO probably macLegacy-related debug('[webRequestOnBeforeRequest] always open in tmpcontainer request, simulating click', request); this.linkClicked(request.url, { @@ -121,40 +140,28 @@ class Request { }); } - if (tab.incognito) { + if (tab && tab.incognito) { debug('[webRequestOnBeforeRequest] tab is incognito, ignore it', tab); return; } - if (!alwaysOpenIn && this.background.noContainerTabs[tab.id]) { + if (!alwaysOpenIn && this.background.noContainerTabs[request.tabId]) { debug('[webRequestOnBeforeRequest] no container tab, we ignore that', tab); return; } - let returnVal; if (this.mouseclick.linksClicked[request.url]) { - returnVal = await this.handleClickedLink(request, tab, macAssignment); + return this.handleClickedLink(request, tab, macAssignment); } else { - returnVal = await this.handleNotClickedLink(request, tab, alwaysOpenIn, macAssignment); + return this.handleNotClickedLink(request, tab, alwaysOpenIn, macAssignment); } - - if (returnVal && returnVal.cancel) { - this.cancelRequest(request); - } else { - if (this.storage.local.tempContainers[tab.cookieStoreId] && - this.storage.local.tempContainers[tab.cookieStoreId].clean) { - debug('[webRequestOnBeforeRequest] marking tmp container as not clean anymore', tab); - this.storage.local.tempContainers[tab.cookieStoreId].clean = false; - } - } - return returnVal; } async handleClickedLink(request, tab, macAssignment) { debug('[handleClickedLink] onBeforeRequest', request, tab); - if (tab.cookieStoreId !== 'firefox-default' && + if (tab && tab.cookieStoreId !== 'firefox-default' && this.container.urlCreatedContainer[request.url] === tab.cookieStoreId) { debug('[handleClickedLink] link click already created this container, we can stop here', request, tab); return; @@ -166,7 +173,8 @@ class Request { } let deletesHistoryContainer = false; - if (this.storage.local.tempContainers[tab.cookieStoreId] && + if (tab && + this.storage.local.tempContainers[tab.cookieStoreId] && this.storage.local.tempContainers[tab.cookieStoreId].deletesHistory && this.storage.local.preferences.deletesHistoryContainerMouseClicks === 'automatic') { deletesHistoryContainer = true; @@ -183,7 +191,7 @@ class Request { this.mouseclick.linksClicked[request.url].clickType === 'left') { debug('[handleClickedLink] creating new container because request got left clicked', this.mouseclick.linksClicked[request.url], tab); newTab = await this.container.createTabInTempContainer({tab, active: true, url: request.url, deletesHistory: deletesHistoryContainer, request}); - if (this.mouseclick.linksClicked[request.url].tab.id !== tab.id) { + if (tab && this.mouseclick.linksClicked[request.url].tab.id !== tab.id) { debug('[handleClickedLink] looks like the left clicked opened a new tab, remove it', tab); await this.container.removeTab(tab); } @@ -197,7 +205,7 @@ class Request { async handleNotClickedLink(request, tab, alwaysOpenIn, macAssignment) { - if (tab.cookieStoreId === 'firefox-default' && tab.openerTabId && !alwaysOpenIn) { + if (tab && tab.cookieStoreId === 'firefox-default' && tab.openerTabId && !alwaysOpenIn) { debug('[webRequestOnBeforeRequest] default container and openerTabId set', tab); const openerTab = await browser.tabs.get(tab.openerTabId); if (!openerTab.url.startsWith('about:') && !openerTab.url.startsWith('moz-extension:')) { @@ -210,22 +218,24 @@ class Request { return; } - let containerExists = false; - if (tab.cookieStoreId === 'firefox-default') { - containerExists = true; - } else { - try { - containerExists = await browser.contextualIdentities.get(tab.cookieStoreId); - } catch (error) { - debug('[handleNotClickedLink] container doesnt exist anymore, probably undo close tab', tab); + if (!macAssignment) { + let containerExists = false; + if (tab.cookieStoreId === 'firefox-default') { + containerExists = true; + } else { + try { + containerExists = await browser.contextualIdentities.get(tab.cookieStoreId); + } catch (error) { + debug('[handleNotClickedLink] container doesnt exist anymore', tab); + } } - } - if (tab.cookieStoreId !== 'firefox-default' && containerExists) { - if (this.shouldCancelRequest(request)) { - return { cancel: true }; + if (tab && tab.cookieStoreId !== 'firefox-default' && containerExists) { + if (this.shouldCancelRequest(request)) { + return { cancel: true }; + } + debug('[handleNotClickedLink] onBeforeRequest tab belongs to a non-default container', tab, request); + return; } - debug('[handleNotClickedLink] onBeforeRequest tab belongs to a non-default container', tab, request); - return; } if (this.cancelRequest(request)) { @@ -240,7 +250,7 @@ class Request { if (macAssignment) { debug('[handleNotClickedLink] decided to reopen but mac assigned, reopen confirmpage', request, tab, macAssignment); this.mac.maybeReopenConfirmPage(macAssignment, request, tab, deletesHistoryContainer); - return; + return { cancel: true }; } debug('[handleNotClickedLink] onBeforeRequest reload in temp tab', tab, request); diff --git a/test/background.mac.test.js b/test/background.mac.test.js index 00adf45a..ca266595 100644 --- a/test/background.mac.test.js +++ b/test/background.mac.test.js @@ -5,12 +5,8 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify( global.background = await loadBackground(preferences); }); - describe('opening new tmptab and loading mac assigned url and not "remember my choice"', () => { + describe('opening new tmptab', () => { beforeEach(async () => { - browser.runtime.sendMessage.resolves({ - userContextId: '1', - neverAsk: false - }); await helper.browser.openNewTmpTab({ tabId: 1, createsTabId: 2, @@ -19,23 +15,159 @@ preferencesTestSet.map(preferences => { describe(`preferences: ${JSON.stringify( await nextTick(); }); - it('should not interrupt since the tmptab is clean', async () => { - helper.browser.request({ - requestId: 1, - tabId: 2, - originContainer: 'firefox-tmp1', - url: 'http://example.com', - resetHistory: true + describe('and opening a mac assigned website with not "remember my choice"', () => { + let originContainer = 'firefox-tmp1'; + if (preferences.automaticModeNewTab === 'navigation') { + originContainer = 'firefox-default'; + } + beforeEach(async () => { + browser.runtime.sendMessage.resolves({ + userContextId: '1', + neverAsk: false + }); + await helper.browser.request({ + requestId: 1, + tabId: 2, + originContainer, + url: 'http://example.com' + }); }); - await helper.browser.openMacConfirmPage({ - tabId: 3, - originContainer: 'firefox-tmp1', - targetContainer: 'firefox-container-1', - url: 'http://example.com' + + it('should do the right thing', async () => { + await helper.browser.openMacConfirmPage({ + tabId: 3, + originContainer, + targetContainer: 'firefox-container-1', + url: 'http://example.com', + resetHistory: true + }); + await nextTick(); + + if (preferences.automaticModeNewTab === 'navigation') { + browser.tabs.remove.should.have.been.called; + browser.tabs.create.should.have.been.called; + } else { + browser.tabs.remove.should.not.have.been.called; + browser.tabs.create.should.not.have.been.called; + } }); + }); + }); + + describe('opening new tmptab', () => { + beforeEach(async () => { + await helper.browser.openNewTmpTab({ + tabId: 1, + createsTabId: 2, + createsContainer: 'firefox-tmp1', + }); + await nextTick(); + }); + + describe('and opening a not mac assigned website', () => { + let newTmpTabId = 2; + beforeEach(async () => { + await helper.browser.request({ + requestId: 1, + tabId: newTmpTabId, + originContainer: 'firefox-tmp1', + url: 'http://example.com', + resetHistory: true + }); + }); + + [ + { + click: { + type: 'middle', + action: 'never' + } + }, + { + click: { + type: 'middle', + action: 'always' + } + }, + { + click: { + type: 'ctrlleft', + action: 'never' + } + }, + { + click: { + type: 'ctrlleft', + action: 'always' + } + }, + { + click: { + type: 'left', + action: 'never' + } + }, + { + click: { + type: 'left', + action: 'always' + } + }, + ].map(preferences => { describe(`preferences: ${JSON.stringify(preferences)}`, () => { + + describe('clicks on links in the loaded website that are mac assigned with not "remember my choice"', () => { + beforeEach(async () => { + background.storage.local.preferences.linkClickGlobal[preferences.click.type].action = preferences.click.action; + let tabId; + switch (preferences.click.action) { + case 'middle': + case 'ctrlleft': + tabId = newTmpTabId+1; + break; + case 'left': + tabId = newTmpTabId; + break; + } + browser.runtime.sendMessage.resolves({ + userContextId: '1', + neverAsk: false + }); + await helper.browser.mouseClickOnLink({ + clickType: preferences.click.type, + senderUrl: 'http://example.com', + targetUrl: 'http://notexample.com', + }); + await helper.browser.request({ + requestId: 2, + tabId, + originContainer: 'firefox-tmp1', + url: 'http://notexample.com', + resetHistory: true + }); + await nextTick(); + }); + + it('should do the right thing', async () => { + await helper.browser.openMacConfirmPage({ + tabId: 3, + originContainer: 'firefox-tmp1', + targetContainer: 'firefox-container-1', + url: 'http://example.com' + }); + await nextTick(); - browser.tabs.remove.should.not.have.been.called; - browser.tabs.create.should.not.have.been.called; + switch (preferences.click.action) { + case 'always': + browser.tabs.remove.should.have.been.called; + browser.tabs.create.should.have.been.called; + break; + case 'never': + browser.tabs.remove.should.not.have.been.called; + browser.tabs.create.should.not.have.been.called; + } + }); + }); + });}); }); }); }); diff --git a/test/helper.js b/test/helper.js index ebdce788..584ba9b6 100644 --- a/test/helper.js +++ b/test/helper.js @@ -76,6 +76,13 @@ module.exports = { let clickEvent = {}; switch (clickType) { + case 'middle': + clickEvent.button = 1; + break; + case 'ctrlleft': + clickEvent.button = 0; + clickEvent.ctrlKey = true; + break; case 'left': clickEvent.button = 0; break; @@ -91,9 +98,7 @@ module.exports = { method: 'linkClicked', payload: { href: targetUrl, - event: { - button: clickEvent.button - } + event: clickEvent } }; return await background.runtimeOnMessage(fakeMessage, fakeSender); @@ -103,12 +108,19 @@ module.exports = { tabId = 1, originContainer = 'firefox-default', url = 'http://example.com', - targetContainer = false + targetContainer = false, + resetHistory = false }) { + if (resetHistory) { + browser.tabs.remove.resetHistory(); + browser.tabs.create.resetHistory(); + browser.contextualIdentities.create.resetHistory(); + } + let confirmPageUrl = 'moz-extension://multi-account-containers/confirm-page.html?url=' + encodeURIComponent(url) + '&cookieStoreId=' + targetContainer; - if (originContainer) { - '¤tCookieStoreId=' + originContainer; + if (originContainer !== 'firefox-default') { + confirmPageUrl += '¤tCookieStoreId=' + originContainer; } const changeInfo = { url: confirmPageUrl