From 496257e3ce5cae715a019be33eb35da30d159cc2 Mon Sep 17 00:00:00 2001 From: joey Date: Tue, 10 Mar 2020 16:32:09 -0400 Subject: [PATCH 1/9] made deleteHandler and updateDeleteButton private --- src/js/popup.js | 12 ++++++------ test/issues/1624.test.js | 0 2 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 test/issues/1624.test.js diff --git a/src/js/popup.js b/src/js/popup.js index 2a2afcd..26230cf 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -990,7 +990,7 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { /* This function is the handler of deletion for both keypress delete and delete button. The keypress delete support for both backspace and delete key. */ - async deleteHandler() { + async _deleteHandler() { const selectedIdentities = Logic.currentSelectedIdentities(); if (selectedIdentities.length > 0) { await Logic.showPanel(P_CONTAINER_DELETE); @@ -1000,7 +1000,7 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { /* The function is to update the delete button. The delete button shows up once any containers are selected. */ - updateDeleteButton(selectedContainers) { + _updateDeleteButton(selectedContainers) { const deleteButton = document.querySelector("div.panel-footer.panel-footer-secondary"); if (selectedContainers.length === 0) { deleteButton.classList.add("hide"); @@ -1015,13 +1015,13 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { Logic.showPanel(P_CONTAINERS_LIST); }); - Logic.addEnterHandler(document.querySelector("#delete-link"), this.deleteHandler); + Logic.addEnterHandler(document.querySelector("#delete-link"), this._deleteHandler); document.addEventListener("keydown", e => { if (e.keyCode === 16) { this.shiftOn = true; } else if (e.keyCode === 8 || e.keyCode === 48) { - this.deleteHandler(); + this._deleteHandler(); } }); @@ -1035,7 +1035,7 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { // This method is called when the panel is shown. prepare() { Logic.resetSelectedIdentities(); - this.updateDeleteButton(Logic.currentSelectedIdentities()); + this._updateDeleteButton(Logic.currentSelectedIdentities()); const fragment = document.createDocumentFragment(); Logic.identities().forEach(identity => { const tr = document.createElement("tr"); @@ -1107,7 +1107,7 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { } this.lastSelected = identity; - this.updateDeleteButton(currentSelectedIdentity); + this._updateDeleteButton(currentSelectedIdentity); } }); }); diff --git a/test/issues/1624.test.js b/test/issues/1624.test.js new file mode 100644 index 0000000..e69de29 From 23d9e758911e8acb9c9117ced12bd5d7ab0277bf Mon Sep 17 00:00:00 2001 From: joey Date: Tue, 10 Mar 2020 21:58:55 -0400 Subject: [PATCH 2/9] fixed that delete keycode is wrong issue --- src/js/popup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/popup.js b/src/js/popup.js index 26230cf..7f3aaa6 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -1020,7 +1020,7 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { document.addEventListener("keydown", e => { if (e.keyCode === 16) { this.shiftOn = true; - } else if (e.keyCode === 8 || e.keyCode === 48) { + } else if (e.keyCode === 8 || e.keyCode === 46) { this._deleteHandler(); } }); From 6fb11803726661cd0e00cfaf069e4183df215bd2 Mon Sep 17 00:00:00 2001 From: joey Date: Tue, 10 Mar 2020 23:58:15 -0400 Subject: [PATCH 3/9] added one sample test case --- test/issues/1624.test.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/issues/1624.test.js b/test/issues/1624.test.js index e69de29..3e88a1e 100644 --- a/test/issues/1624.test.js +++ b/test/issues/1624.test.js @@ -0,0 +1,35 @@ +const {initializeWithTab} = require("../common"); + +describe("Delete multiple Containers", function () { + beforeEach(async function () { + this.webExt = await initializeWithTab(); + }); + + afterEach(function () { + this.webExt.destroy(); + }); + + describe("creating a new container", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("container-add-link"); + await this.webExt.popup.helper.clickElementById("edit-container-ok-link"); + }); + + it("should create it in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.create.should.have.been.calledOnce; + }); + + describe("manually select one container and delete by delete button", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("edit-containers-link"); + await this.webExt.popup.helper.clickElementByQuerySelectorAll(".edit-container-icon", "last"); + await this.webExt.popup.helper.clickElementById("delete-link", "last"); + await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + }); + + it("should remove it in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledOnce; + }); + }); + }); +}); \ No newline at end of file From 75045a0ba954bcb94a45959f5edc170342583015 Mon Sep 17 00:00:00 2001 From: QiiLin Date: Wed, 11 Mar 2020 11:08:06 -0400 Subject: [PATCH 4/9] update confirm --- src/js/popup.js | 39 ++++++++++++++++++++++++++------------- src/popup.html | 2 +- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/js/popup.js b/src/js/popup.js index 2a2afcd..b29ba51 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -1329,6 +1329,9 @@ Logic.registerPanel(P_CONTAINER_DELETE, { prepare() { // if current identity is not null, then show single container information, otherwise show all selected let currentSelection; + let totalNumberOfTabs = 0; + let containerString = ""; + try { currentSelection = [Logic.currentIdentity()]; } catch (e) { @@ -1336,22 +1339,32 @@ Logic.registerPanel(P_CONTAINER_DELETE, { } // right now for mult-selection, it displays the first item in the selection at the icon and name // Populating the panel: name, icon, and warning message - document.getElementById("delete-container-name").textContent = currentSelection[0].name; - document.getElementById("delete-container-tab-warning").textContent = ""; + document.getElementById("delete-container-tab-warning").textContent = ``; for (let i = 0; i < currentSelection.length; i++) { const identity = currentSelection[i]; - const totalNumberOfTabs = identity.numberOfHiddenTabs + identity.numberOfOpenTabs; - let warningMessage = ""; - if (totalNumberOfTabs > 0) { - const grammaticalNumTabs = totalNumberOfTabs > 1 ? "tabs" : "tab"; - warningMessage =`If you remove ${identity.name} container now, ${totalNumberOfTabs} container ${grammaticalNumTabs} will be closed.`; - } - document.getElementById("delete-container-tab-warning").textContent += warningMessage; - - const icon = document.getElementById("delete-container-icon"); - icon.setAttribute("data-identity-icon", identity.icon); - icon.setAttribute("data-identity-color", identity.color); + totalNumberOfTabs += identity.numberOfHiddenTabs + identity.numberOfOpenTabs; } + console.log(currentSelection.length); + const icon = document.getElementById("delete-container-icon"); + if (currentSelection.length === 1 ) { + document.getElementById("delete-container-name").textContent = currentSelection[0].name; + icon.style.visibility = 'visible'; + icon.style.marginLeft = `0px`; + icon.setAttribute("data-identity-icon", currentSelection[0].icon); + icon.setAttribute("data-identity-color", currentSelection[0].color); + containerString = "this container"; + } else { + icon.style.visibility = 'hidden'; + icon.style.marginLeft = `-16px`; + document.getElementById("delete-container-name").textContent = `Containers`; + containerString = "those " + currentSelection.length + " containers"; + } + let warningMessage = ""; + if (totalNumberOfTabs > 0) { + const grammaticalNumTabs = totalNumberOfTabs > 1 ? "tabs" : "tab"; + warningMessage =`If you remove ${containerString} now, ${totalNumberOfTabs} container ${grammaticalNumTabs} will be closed.`; + } + document.getElementById("delete-container-tab-warning").textContent += warningMessage + ` Are you sure you want to remove ${containerString}?`; return Promise.resolve(null); }, }); diff --git a/src/popup.html b/src/popup.html index 8e03d4d..6cae7fd 100644 --- a/src/popup.html +++ b/src/popup.html @@ -228,7 +228,7 @@

Remove This Container

-

Are you sure you want to remove this Container?

+

Cancel From 0ede2b9d1da243b58f3663192e8d201aa529288c Mon Sep 17 00:00:00 2001 From: joey Date: Wed, 11 Mar 2020 11:11:16 -0400 Subject: [PATCH 5/9] updates test cases --- test/issues/1624.test.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/issues/1624.test.js b/test/issues/1624.test.js index 3e88a1e..d287409 100644 --- a/test/issues/1624.test.js +++ b/test/issues/1624.test.js @@ -23,12 +23,28 @@ describe("Delete multiple Containers", function () { beforeEach(async function () { await this.webExt.popup.helper.clickElementById("edit-containers-link"); await this.webExt.popup.helper.clickElementByQuerySelectorAll(".edit-container-icon", "last"); - await this.webExt.popup.helper.clickElementById("delete-link", "last"); + await this.webExt.popup.helper.clickElementById("delete-link"); await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); }); it("should remove it in the browser as well", function () { - this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledOnce; + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-5"); + }); + }); + + describe("manually click select multiple contaienr and delete by delete button", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("container-add-link"); + await this.webExt.popup.helper.clickElementById("edit-container-ok-link"); + + await this.webExt.popup.helper.clickElementById("edit-containers-link"); + // TODO: select multi containers here + await this.webExt.popup.helper.clickElementById("delete-link"); + await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + }); + + it("should remove it in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-6"); }); }); }); From e5653a0b6eba78e8103bf3f6d688cedc68796b8f Mon Sep 17 00:00:00 2001 From: QiiLin Date: Wed, 11 Mar 2020 12:18:28 -0400 Subject: [PATCH 6/9] update wording --- src/js/popup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/popup.js b/src/js/popup.js index b29ba51..9e786a2 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -1357,7 +1357,7 @@ Logic.registerPanel(P_CONTAINER_DELETE, { icon.style.visibility = 'hidden'; icon.style.marginLeft = `-16px`; document.getElementById("delete-container-name").textContent = `Containers`; - containerString = "those " + currentSelection.length + " containers"; + containerString = "this " + currentSelection.length + " containers"; } let warningMessage = ""; if (totalNumberOfTabs > 0) { From caae012de08a288179f6552577a681b60d2396a0 Mon Sep 17 00:00:00 2001 From: joey Date: Wed, 11 Mar 2020 12:31:14 -0400 Subject: [PATCH 7/9] added manually select and delete multilple containers --- test/issues/1624.test.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/test/issues/1624.test.js b/test/issues/1624.test.js index d287409..155f75c 100644 --- a/test/issues/1624.test.js +++ b/test/issues/1624.test.js @@ -11,40 +11,47 @@ describe("Delete multiple Containers", function () { describe("creating a new container", function () { beforeEach(async function () { - await this.webExt.popup.helper.clickElementById("container-add-link"); - await this.webExt.popup.helper.clickElementById("edit-container-ok-link"); + for (let i = 0; i < 3; i++) { + await this.webExt.popup.helper.clickElementById("container-add-link"); + await this.webExt.popup.helper.clickElementById("edit-container-ok-link"); + } }); it("should create it in the browser as well", function () { - this.webExt.background.browser.contextualIdentities.create.should.have.been.calledOnce; + this.webExt.background.browser.contextualIdentities.create.should.have.been.calledThrice; }); describe("manually select one container and delete by delete button", function () { beforeEach(async function () { await this.webExt.popup.helper.clickElementById("edit-containers-link"); - await this.webExt.popup.helper.clickElementByQuerySelectorAll(".edit-container-icon", "last"); + await this.webExt.popup.helper.clickElementByQuerySelectorAll(".select-container", "last"); await this.webExt.popup.helper.clickElementById("delete-link"); await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + + await this.webExt.popup.helper.clickElementByQuerySelectorAll(".select-container", "last"); + }); it("should remove it in the browser as well", function () { - this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-5"); + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-7"); }); }); describe("manually click select multiple contaienr and delete by delete button", function () { beforeEach(async function () { - await this.webExt.popup.helper.clickElementById("container-add-link"); - await this.webExt.popup.helper.clickElementById("edit-container-ok-link"); - await this.webExt.popup.helper.clickElementById("edit-containers-link"); - // TODO: select multi containers here + + const nodeArray = Array.from(this.webExt.popup.window.document.querySelectorAll(".select-container")); + nodeArray[5].click(); + nodeArray[6].click(); + await this.webExt.popup.helper.clickElementById("delete-link"); await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + }); it("should remove it in the browser as well", function () { - this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-6"); + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledTwice; }); }); }); From 297200bd4fc8c99114d7f508b9d2541e05f69e8d Mon Sep 17 00:00:00 2001 From: joey Date: Wed, 11 Mar 2020 13:07:27 -0400 Subject: [PATCH 8/9] added test cases for shift selct, backspace key, and delete key --- test/issues/1624.test.js | 88 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 5 deletions(-) diff --git a/test/issues/1624.test.js b/test/issues/1624.test.js index 155f75c..429c6f9 100644 --- a/test/issues/1624.test.js +++ b/test/issues/1624.test.js @@ -1,6 +1,6 @@ const {initializeWithTab} = require("../common"); -describe("Delete multiple Containers", function () { +describe("Remove multiple Containers", function () { beforeEach(async function () { this.webExt = await initializeWithTab(); }); @@ -9,7 +9,7 @@ describe("Delete multiple Containers", function () { this.webExt.destroy(); }); - describe("creating a new container", function () { + describe("creating three new containers", function () { beforeEach(async function () { for (let i = 0; i < 3; i++) { await this.webExt.popup.helper.clickElementById("container-add-link"); @@ -17,7 +17,7 @@ describe("Delete multiple Containers", function () { } }); - it("should create it in the browser as well", function () { + it("should create these in the browser as well", function () { this.webExt.background.browser.contextualIdentities.create.should.have.been.calledThrice; }); @@ -27,9 +27,25 @@ describe("Delete multiple Containers", function () { await this.webExt.popup.helper.clickElementByQuerySelectorAll(".select-container", "last"); await this.webExt.popup.helper.clickElementById("delete-link"); await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + }); + it("should remove it in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-7"); + }); + }); + + describe("manually select one container and delete by backspace key", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("edit-containers-link"); await this.webExt.popup.helper.clickElementByQuerySelectorAll(".select-container", "last"); + const backspaceKey = 6; + const event = new this.webExt.popup.window.KeyboardEvent("keydown",{"keyCode": backspaceKey}); + this.webExt.popup.window.document.dispatchEvent(event); + + await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + + }); it("should remove it in the browser as well", function () { @@ -37,7 +53,26 @@ describe("Delete multiple Containers", function () { }); }); - describe("manually click select multiple contaienr and delete by delete button", function () { + describe("manually select one container and delete by delete key", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("edit-containers-link"); + await this.webExt.popup.helper.clickElementByQuerySelectorAll(".select-container", "last"); + + const deleteKey = 46; + const event = new this.webExt.popup.window.KeyboardEvent("keydown",{"keyCode": deleteKey}); + this.webExt.popup.window.document.dispatchEvent(event); + + await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + + + }); + + it("should remove it in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-7"); + }); + }); + + describe("manually click select two containers and delete by delete button", function () { beforeEach(async function () { await this.webExt.popup.helper.clickElementById("edit-containers-link"); @@ -50,9 +85,52 @@ describe("Delete multiple Containers", function () { }); - it("should remove it in the browser as well", function () { + it("should remove it in the browser twice as well", function () { this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledTwice; }); + + it("should remove the container # 7 as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-7"); + }); + + it("should remove the container # 6 as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-6"); + }); + }); + + describe("manually shift click select multiple containers and delete by delete button", function () { + beforeEach(async function () { + await this.webExt.popup.helper.clickElementById("edit-containers-link"); + + const nodeArray = Array.from(this.webExt.popup.window.document.querySelectorAll(".select-container")); + nodeArray[4].click(); + + const shiftKey = 16; + const event = new this.webExt.popup.window.KeyboardEvent("keydown",{"keyCode": shiftKey}); + this.webExt.popup.window.document.dispatchEvent(event); + + nodeArray[6].click(); + + await this.webExt.popup.helper.clickElementById("delete-link"); + await this.webExt.popup.helper.clickElementById("delete-container-ok-link"); + + }); + + it("should remove these three containers in the browser as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledThrice; + }); + + it("should remove the container # 5 as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-5"); + }); + + it("should remove the container # 6 as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-6"); + }); + + it("should remove the container # 7 as well", function () { + this.webExt.background.browser.contextualIdentities.remove.should.have.been.calledWith("firefox-container-7"); + }); }); }); }); \ No newline at end of file From 1b549adc75fea020f46aa7af9d62a4da13207e27 Mon Sep 17 00:00:00 2001 From: QiiLin <36078357+QiiLin@users.noreply.github.com> Date: Wed, 11 Mar 2020 13:19:34 -0400 Subject: [PATCH 9/9] Delete console.log --- src/js/popup.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/popup.js b/src/js/popup.js index 49ebccd..ab0904a 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -1344,7 +1344,6 @@ Logic.registerPanel(P_CONTAINER_DELETE, { const identity = currentSelection[i]; totalNumberOfTabs += identity.numberOfHiddenTabs + identity.numberOfOpenTabs; } - console.log(currentSelection.length); const icon = document.getElementById("delete-container-icon"); if (currentSelection.length === 1 ) { document.getElementById("delete-container-name").textContent = currentSelection[0].name;