diff options
author | Andrei Oprea <andrei.br92@gmail.com> | 2019-06-05 16:36:44 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-05 16:36:44 +0000 |
commit | 450d637cfd54c72bc525da6dd28e2155f5bd55d4 (patch) | |
tree | c15fc8e1d6b958ff346fb83186bcc29f0338622f | |
parent | 61746983b59d3b4daad5a18a2c3e5f5184755a1e (diff) |
Bug 1552809 - Enable Bookmark panel messages to work without translations (#5067)
-rw-r--r-- | lib/ASRouter.jsm | 2 | ||||
-rw-r--r-- | lib/BookmarkPanelHub.jsm | 68 | ||||
-rw-r--r-- | lib/PanelTestProvider.jsm | 33 | ||||
-rw-r--r-- | test/unit/asrouter/PanelTestProvider.test.js | 2 | ||||
-rw-r--r-- | test/unit/lib/BookmarkPanelHub.test.js | 96 |
5 files changed, 141 insertions, 60 deletions
diff --git a/lib/ASRouter.jsm b/lib/ASRouter.jsm index 7db9f5fc..40ee8788 100644 --- a/lib/ASRouter.jsm +++ b/lib/ASRouter.jsm @@ -987,7 +987,7 @@ class _ASRouter { break; case "fxa_bookmark_panel": if (force) { - BookmarkPanelHub._forceShowMessage(message); + BookmarkPanelHub._forceShowMessage(target, message); } break; default: diff --git a/lib/BookmarkPanelHub.jsm b/lib/BookmarkPanelHub.jsm index 98de50cc..5609c5f3 100644 --- a/lib/BookmarkPanelHub.jsm +++ b/lib/BookmarkPanelHub.jsm @@ -70,6 +70,9 @@ class _BookmarkPanelHub { return true; } + // If we didn't match on a previously cached request then make sure + // the container is empty + this._removeContainer(target); const response = await this._handleMessageRequest(this._trigger); return this.onResponse(response, target, win); @@ -105,7 +108,7 @@ class _BookmarkPanelHub { } showMessage(message, target, win) { - if (this._response.collapsed) { + if (this._response && this._response.collapsed) { this.toggleRecommendation(false); return; } @@ -131,7 +134,6 @@ class _BookmarkPanelHub { close.setAttribute("id", "cfrClose"); close.setAttribute("aria-label", "close"); close.style.color = message.color; - this._l10n.setAttributes(close, message.close_button.tooltiptext); close.addEventListener("click", e => { this.sendUserEventTelemetry("DISMISS", win); this.collapseMessage(); @@ -139,13 +141,24 @@ class _BookmarkPanelHub { }); const title = createElement("h1"); title.setAttribute("id", "editBookmarkPanelRecommendationTitle"); - this._l10n.setAttributes(title, message.title); const content = createElement("p"); content.setAttribute("id", "editBookmarkPanelRecommendationContent"); - this._l10n.setAttributes(content, message.text); const cta = createElement("button"); cta.setAttribute("id", "editBookmarkPanelRecommendationCta"); - this._l10n.setAttributes(cta, message.cta); + + // If `string_id` is present it means we are relying on fluent for translations + if (message.text.string_id) { + this._l10n.setAttributes(close, message.close_button.tooltiptext.string_id); + this._l10n.setAttributes(title, message.title.string_id); + this._l10n.setAttributes(content, message.text.string_id); + this._l10n.setAttributes(cta, message.cta.string_id); + } else { + close.setAttribute("title", message.close_button.tooltiptext); + title.textContent = message.title; + content.textContent = message.text; + cta.textContent = message.cta; + } + recommendation.appendChild(close); recommendation.appendChild(title); recommendation.appendChild(content); @@ -157,6 +170,10 @@ class _BookmarkPanelHub { } toggleRecommendation(visible) { + if (!this._response) { + return; + } + const {target} = this._response; if (visible === undefined) { // When called from the info button of the bookmark panel @@ -167,9 +184,9 @@ class _BookmarkPanelHub { if (target.infoButton.checked) { // If it was ever collapsed we need to cancel the state this._response.collapsed = false; - target.recommendationContainer.removeAttribute("disabled"); + target.container.removeAttribute("disabled"); } else { - target.recommendationContainer.setAttribute("disabled", "disabled"); + target.container.setAttribute("disabled", "disabled"); } } @@ -178,18 +195,41 @@ class _BookmarkPanelHub { this.toggleRecommendation(false); } - hideMessage(target) { - const container = target.container.querySelector("#cfrMessageContainer"); - if (container) { - container.remove(); + _removeContainer(target) { + if (target || (this._response && this._response.target)) { + const container = (target || this._response.target).container.querySelector("#cfrMessageContainer"); + if (container) { + container.remove(); + } } + } + + hideMessage(target) { + this._removeContainer(target); this.toggleRecommendation(false); this._response = null; } - _forceShowMessage(message) { - this.toggleRecommendation(true); - this.showMessage(message.content, this._response.target, this._response.win); + _forceShowMessage(target, message) { + const doc = target.browser.ownerGlobal.gBrowser.ownerDocument; + const win = target.browser.ownerGlobal.window; + const panelTarget = { + container: doc.getElementById("editBookmarkPanelRecommendation"), + infoButton: doc.getElementById("editBookmarkPanelInfoButton"), + document: doc, + close: e => { + e.stopPropagation(); + this.toggleRecommendation(false); + }, + }; + // Remove any existing message + this.hideMessage(panelTarget); + // Reset the reference to the panel elements + this._response = {target: panelTarget}; + // Required if we want to preview messages that include fluent strings + win.MozXULElement.insertFTLIfNeeded("browser/newtab/asrouter.ftl"); + win.MozXULElement.insertFTLIfNeeded("browser/branding/sync-brand.ftl"); + this.showMessage(message.content, panelTarget, win); } sendImpression() { diff --git a/lib/PanelTestProvider.jsm b/lib/PanelTestProvider.jsm index ba35aaad..a85c222c 100644 --- a/lib/PanelTestProvider.jsm +++ b/lib/PanelTestProvider.jsm @@ -5,20 +5,39 @@ const MESSAGES = () => ([ { - "id": "SIMPLE_FXA_BOOKMARK_TEST_1", + "id": "SIMPLE_FXA_BOOKMARK_TEST_FLUENT", "template": "fxa_bookmark_panel", "content": { - "title": "cfr-doorhanger-bookmark-fxa-header", - "text": "cfr-doorhanger-bookmark-fxa-body", - "cta": "cfr-doorhanger-bookmark-fxa-link-text", + "title": {"string_id": "cfr-doorhanger-bookmark-fxa-header"}, + "text": {"string_id": "cfr-doorhanger-bookmark-fxa-body"}, + "cta": {"string_id": "cfr-doorhanger-bookmark-fxa-link-text"}, "color": "white", "background_color_1": "#7d31ae", "background_color_2": "#5033be", "info_icon": { - "tooltiptext": "cfr-doorhanger-bookmark-fxa-info-icon-tooltip", + "tooltiptext": {"string_id": "cfr-doorhanger-bookmark-fxa-info-icon-tooltip"}, }, "close_button": { - "tooltiptext": "cfr-doorhanger-bookmark-fxa-close-btn-tooltip", + "tooltiptext": {"string_id": "cfr-doorhanger-bookmark-fxa-close-btn-tooltip"}, + }, + }, + "trigger": {"id": "bookmark-panel"}, + }, + { + "id": "SIMPLE_FXA_BOOKMARK_TEST_NON_FLUENT", + "template": "fxa_bookmark_panel", + "content": { + "title": "Bookmark Message Title", + "text": "Bookmark Message Body", + "cta": "Sync bookmarks now", + "color": "white", + "background_color_1": "#7d31ae", + "background_color_2": "#5033be", + "info_icon": { + "tooltiptext": "Toggle tooltip", + }, + "close_button": { + "tooltiptext": "Close tooltip", }, }, "trigger": {"id": "bookmark-panel"}, @@ -28,7 +47,7 @@ const MESSAGES = () => ([ const PanelTestProvider = { getMessages() { return MESSAGES() - .map(message => ({...message, targeting: `true`})); + .map(message => ({...message, targeting: `providerCohorts.panel_local_testing == "SHOW_TEST"`})); }, }; this.PanelTestProvider = PanelTestProvider; diff --git a/test/unit/asrouter/PanelTestProvider.test.js b/test/unit/asrouter/PanelTestProvider.test.js index b9842004..e1b62c6c 100644 --- a/test/unit/asrouter/PanelTestProvider.test.js +++ b/test/unit/asrouter/PanelTestProvider.test.js @@ -4,7 +4,7 @@ const messages = PanelTestProvider.getMessages(); describe("PanelTestProvider", () => { it("should have a message", () => { - assert.lengthOf(messages, 1); + assert.lengthOf(messages, 2); }); it("should be a valid message", () => { assert.jsonSchema(messages[0].content, schema); diff --git a/test/unit/lib/BookmarkPanelHub.test.js b/test/unit/lib/BookmarkPanelHub.test.js index 87cec448..2a04daef 100644 --- a/test/unit/lib/BookmarkPanelHub.test.js +++ b/test/unit/lib/BookmarkPanelHub.test.js @@ -1,5 +1,6 @@ import {_BookmarkPanelHub} from "lib/BookmarkPanelHub.jsm"; import {GlobalOverrider} from "test/unit/utils"; +import {PanelTestProvider} from "lib/PanelTestProvider.jsm"; describe("BookmarkPanelHub", () => { let globals; @@ -9,6 +10,7 @@ describe("BookmarkPanelHub", () => { let fakeHandleMessageRequest; let fakeL10n; let fakeMessage; + let fakeMessageFluent; let fakeTarget; let fakeContainer; let fakeDispatch; @@ -27,44 +29,39 @@ describe("BookmarkPanelHub", () => { instance = new _BookmarkPanelHub(); fakeAddImpression = sandbox.stub(); fakeHandleMessageRequest = sandbox.stub(); - fakeMessage = { - text: "text", - title: "title", - link: { - url: "url", - text: "text", - }, - color: "white", - background_color_1: "#7d31ae", - background_color_2: "#5033be", - info_icon: {tooltiptext: "cfr-bookmark-tooltip-text"}, - close_button: {tooltiptext: "cfr-bookmark-tooltip-text"}, - }; + [{content: fakeMessageFluent}, {content: fakeMessage}] = PanelTestProvider.getMessages(); fakeContainer = { addEventListener: sandbox.stub(), setAttribute: sandbox.stub(), + removeAttribute: sandbox.stub(), classList: {add: sandbox.stub()}, appendChild: sandbox.stub(), + querySelector: sandbox.stub(), children: [], style: {}, }; + fakeWindow = { + ownerGlobal: {openLinkIn: sandbox.stub(), gBrowser: {selectedBrowser: "browser"}}, + MozXULElement: {insertFTLIfNeeded: sandbox.stub()}, + }; + const document = { + createElementNS: sandbox.stub().returns(fakeContainer), + getElementById: sandbox.stub().returns(fakeContainer), + }; fakeTarget = { - document: { - createElementNS: sandbox.stub().returns(fakeContainer), - }, + document, container: { querySelector: sandbox.stub(), appendChild: sandbox.stub(), + setAttribute: sandbox.stub(), + removeAttribute: sandbox.stub(), }, hidePopup: sandbox.stub(), infoButton: {}, close: sandbox.stub(), + browser: {ownerGlobal: {gBrowser: {ownerDocument: document}, window: fakeWindow}}, }; fakeDispatch = sandbox.stub(); - fakeWindow = { - ownerGlobal: {openLinkIn: sandbox.stub(), gBrowser: {selectedBrowser: "browser"}}, - MozXULElement: {insertFTLIfNeeded: sandbox.stub()}, - }; }); afterEach(() => { instance.uninit(); @@ -194,6 +191,22 @@ describe("BookmarkPanelHub", () => { assert.equal(fakeTarget.document.createElementNS.callCount, 5); assert.calledOnce(fakeTarget.container.appendChild); + assert.notCalled(fakeL10n.setAttributes); + }); + it("should create a container (fluent message)", () => { + fakeTarget.container.querySelector.returns(false); + + instance.showMessage(fakeMessageFluent, fakeTarget); + + assert.equal(fakeTarget.document.createElementNS.callCount, 5); + assert.calledOnce(fakeTarget.container.appendChild); + }); + it("should set l10n attributes", () => { + fakeTarget.container.querySelector.returns(false); + + instance.showMessage(fakeMessageFluent, fakeTarget); + + assert.equal(fakeL10n.setAttributes.callCount, 4); }); it("should reuse the container", () => { fakeTarget.container.querySelector.returns(true); @@ -304,7 +317,7 @@ describe("BookmarkPanelHub", () => { beforeEach(() => { target = { infoButton: {}, - recommendationContainer: { + container: { setAttribute: sandbox.stub(), removeAttribute: sandbox.stub(), }, @@ -328,43 +341,52 @@ describe("BookmarkPanelHub", () => { assert.isFalse(target.infoButton.checked); }); - it("should disable recommendationContainer", () => { + it("should disable the container", () => { target.infoButton.checked = true; instance.toggleRecommendation(); - assert.calledOnce(target.recommendationContainer.setAttribute); + assert.calledOnce(target.container.setAttribute); }); - it("should enable recommendationContainer", () => { + it("should enable container", () => { target.infoButton.checked = false; instance.toggleRecommendation(); - assert.calledOnce(target.recommendationContainer.removeAttribute); + assert.calledOnce(target.container.removeAttribute); }); }); describe("#_forceShowMessage", () => { it("should call showMessage with the correct args", () => { - const msg = {content: "foo"}; - const target = {infoButton: {disabled: false}, recommendationContainer: {removeAttribute: sandbox.stub()}}; - sandbox.stub(instance, "showMessage"); - sandbox.stub(instance, "_response").value({target, win: "win"}); + sandbox.spy(instance, "showMessage"); + sandbox.stub(instance, "hideMessage"); - instance._forceShowMessage(msg); + instance._forceShowMessage(fakeTarget, {content: fakeMessage}); assert.calledOnce(instance.showMessage); - assert.calledWithExactly(instance.showMessage, "foo", target, "win"); + assert.calledOnce(instance.hideMessage); + assert.calledWithExactly(instance.showMessage, fakeMessage, sinon.match.object, fakeWindow); }); - it("should call toggleRecommendation with true", () => { - const msg = {content: "foo"}; + it("should insert required fluent files", () => { sandbox.stub(instance, "showMessage"); + + instance._forceShowMessage(fakeTarget, {content: fakeMessage}); + + assert.calledTwice(fakeWindow.MozXULElement.insertFTLIfNeeded); + }); + it("should insert a message you can collapse", () => { + sandbox.spy(instance, "showMessage"); sandbox.stub(instance, "toggleRecommendation"); - sandbox.stub(instance, "_response").value({fakeTarget, win: "win"}); + sandbox.stub(instance, "sendUserEventTelemetry"); - instance._forceShowMessage(msg); + instance._forceShowMessage(fakeTarget, {content: fakeMessage}); - assert.calledOnce(instance.toggleRecommendation); - assert.calledWithExactly(instance.toggleRecommendation, true); + const [, eventListenerCb] = fakeContainer.addEventListener.secondCall.args; + // Called with `true` to show the message + instance.toggleRecommendation.reset(); + eventListenerCb({stopPropagation: sandbox.stub()}); + + assert.calledWithExactly(instance.toggleRecommendation, false); }); }); describe("#sendImpression", () => { |