Bug 1631415 - Reuse the remote-text custom element for CFR layouts r=k88hudson
☠☠ backed out by a58cc68b0c51 ☠ ☠
authorAndrei Oprea <andrei.br92@gmail.com>
Fri, 29 May 2020 16:40:59 +0000
changeset 533032 3a362a5d8b877e7ed26cdd5e61a13b1ea00f8cb1
parent 533031 2f666a44162eff149b94df98d29a8672161cd3c1
child 533033 6ad5f406fb732c243e42e7ffb8b5e11502452516
push id37461
push userccoroiu@mozilla.com
push dateFri, 29 May 2020 21:46:31 +0000
treeherdermozilla-central@a58cc68b0c51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk88hudson
bugs1631415
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1631415 - Reuse the remote-text custom element for CFR layouts r=k88hudson Differential Revision: https://phabricator.services.mozilla.com/D72380
browser/components/newtab/components/CustomElements/paragraph.js
browser/components/newtab/lib/ASRouter.jsm
browser/components/newtab/lib/CFRPageActions.jsm
browser/components/newtab/lib/RemoteL10n.jsm
browser/components/newtab/lib/ToolbarPanelHub.jsm
browser/components/newtab/test/browser/browser_asrouter_cfr.js
--- a/browser/components/newtab/components/CustomElements/paragraph.js
+++ b/browser/components/newtab/components/CustomElements/paragraph.js
@@ -16,19 +16,24 @@
 
       this._content = null;
     }
 
     get fluentAttributeValues() {
       const attributes = {};
       for (let name of this.getAttributeNames()) {
         if (name.startsWith("fluent-variable-")) {
-          attributes[name.replace(/^fluent-variable-/, "")] = this.getAttribute(
-            name
-          );
+          let value = this.getAttribute(name);
+          // Attribute value is a string, in some cases that is not useful
+          // for example instantiating a Date object will fail. We try to
+          // convert all possible integers back.
+          if (value.match(/^\d+/)) {
+            value = parseInt(value, 10);
+          }
+          attributes[name.replace(/^fluent-variable-/, "")] = value;
         }
       }
 
       return attributes;
     }
 
     render() {
       if (this.getAttribute("fluent-remote-id") && this._content) {
--- a/browser/components/newtab/lib/ASRouter.jsm
+++ b/browser/components/newtab/lib/ASRouter.jsm
@@ -1343,16 +1343,17 @@ class _ASRouter {
   routeMessageToTarget(message, target, trigger, force = false) {
     switch (message.template) {
       case "whatsnew_panel_message":
         if (force) {
           ToolbarPanelHub.forceShowMessage(target, message);
         }
         break;
       case "cfr_doorhanger":
+      case "milestone_message":
         if (force) {
           CFRPageActions.forceRecommendation(target, message, this.dispatch);
         } else {
           CFRPageActions.addRecommendation(
             target,
             trigger.param && trigger.param.host,
             message,
             this.dispatch
@@ -1377,19 +1378,16 @@ class _ASRouter {
         }
         break;
       case "toolbar_badge":
         ToolbarBadgeHub.registerBadgeNotificationListener(message, { force });
         break;
       case "update_action":
         MomentsPageHub.executeAction(message);
         break;
-      case "milestone_message":
-        CFRPageActions.showMilestone(target, message, this.dispatch, { force });
-        break;
       default:
         try {
           target.sendAsyncMessage(OUTGOING_MESSAGE_NAME, {
             type: "SET_MESSAGE",
             data: message,
           });
         } catch (e) {}
         break;
--- a/browser/components/newtab/lib/CFRPageActions.jsm
+++ b/browser/components/newtab/lib/CFRPageActions.jsm
@@ -302,16 +302,25 @@ class PageAction {
 
   _blockMessage(messageID) {
     this._dispatchToASRouter({
       type: "BLOCK_MESSAGE_BY_ID",
       data: { id: messageID },
     });
   }
 
+  maybeLoadCustomElement(win) {
+    if (!win.customElements.get("remote-text")) {
+      Services.scriptloader.loadSubScript(
+        "resource://activity-stream/data/custom-elements/paragraph.js",
+        win
+      );
+    }
+  }
+
   /**
    * getStrings - Handles getting the localized strings vs message overrides.
    *              If string_id is not defined it assumes you passed in an override
    *              message and it just returns it.
    *              If subAttribute is provided, the string for it is returned.
    * @return A string. One of 1) passed in string 2) a String object with
    *         attributes property if there are attributes 3) the sub attribute.
    */
@@ -503,16 +512,18 @@ class PageAction {
           });
         }
       };
       animationButton.addEventListener("click", this.onAnimationButtonClick);
     }
   }
 
   async _renderMilestonePopup(message, browser) {
+    this.maybeLoadCustomElement(this.window);
+
     let { content, id } = message;
     let { primary } = content.buttons;
 
     let dateFormat = new Services.intl.DateTimeFormat(
       this.window.gBrowser.ownerGlobal.navigator.language,
       {
         month: "long",
         year: "numeric",
@@ -527,31 +538,28 @@ class PageAction {
     );
     let reachedMilestone = 0;
     let totalSaved = await TrackingDBService.sumAllEvents();
     for (let milestone of milestones) {
       if (totalSaved >= milestone) {
         reachedMilestone = milestone;
       }
     }
-    if (typeof message.content.heading_text === "string") {
-      // This is a test environment.
-      panelTitle = message.content.heading_text;
-      headerLabel.value = panelTitle;
-    } else {
-      RemoteL10n.l10n.setAttributes(
-        headerLabel,
-        content.heading_text.string_id,
-        {
+    if (headerLabel.firstChild) {
+      headerLabel.firstChild.remove();
+    }
+    headerLabel.appendChild(
+      RemoteL10n.createElement(this.window.document, "span", {
+        content: message.content.heading_text,
+        attributes: {
           blockedCount: reachedMilestone,
           date: monthName,
-        }
-      );
-      await RemoteL10n.l10n.translateElements([headerLabel]);
-    }
+        },
+      })
+    );
 
     // Use the message layout as a CSS selector to hide different parts of the
     // notification template markup
     this.window.document
       .getElementById("contextual-feature-recommendation-notification")
       .setAttribute("data-notification-category", content.layout);
     this.window.document
       .getElementById("contextual-feature-recommendation-notification")
@@ -626,16 +634,18 @@ class PageAction {
     Services.prefs.setStringPref(
       "browser.contentblocking.cfr-milestone.milestone-shown-time",
       Date.now().toString()
     );
   }
 
   // eslint-disable-next-line max-statements
   async _renderPopup(message, browser) {
+    this.maybeLoadCustomElement(this.window);
+
     const { id, content, modelVersion } = message;
 
     const headerLabel = this.window.document.getElementById(
       "cfr-notification-header-label"
     );
     const headerLink = this.window.document.getElementById(
       "cfr-notification-header-link"
     );
@@ -678,17 +688,24 @@ class PageAction {
       .getElementById("contextual-feature-recommendation-notification")
       .setAttribute("data-notification-bucket", content.bucket_id);
 
     switch (content.layout) {
       case "icon_and_message":
         const author = this.window.document.getElementById(
           "cfr-notification-author"
         );
-        author.textContent = await this.getStrings(content.text);
+        if (author.firstChild) {
+          author.firstChild.remove();
+        }
+        author.appendChild(
+          RemoteL10n.createElement(this.window.document, "span", {
+            content: content.text,
+          })
+        );
         primaryActionCallback = () => {
           this._blockMessage(id);
           this.dispatchUserAction(primary.action);
           this.hideAddressBarNotifier();
           this._sendTelemetry({
             message_id: id,
             bucket_id: content.bucket_id,
             event: "ENABLE",
@@ -743,29 +760,39 @@ class PageAction {
           } else {
             stepsContainer = this.window.document.createXULElement("vbox");
             stepsContainer.setAttribute("id", stepsContainerId);
           }
           footerText.parentNode.appendChild(stepsContainer);
           for (let step of content.descriptionDetails.steps) {
             // This li is a generic xul element with custom styling
             const li = this.window.document.createXULElement("li");
-            RemoteL10n.l10n.setAttributes(li, step.string_id);
+            li.appendChild(
+              RemoteL10n.createElement(this.window.document, "span", {
+                content: step,
+              })
+            );
             stepsContainer.appendChild(li);
           }
-          await RemoteL10n.l10n.translateElements([...stepsContainer.children]);
         }
 
         await this._renderPinTabAnimation();
         break;
       default:
         panelTitle = await this.getStrings(content.addon.title);
         await this._setAddonAuthorAndRating(this.window.document, content);
+        if (footerText.firstChild) {
+          footerText.firstChild.remove();
+        }
         // Main body content of the dropdown
-        footerText.textContent = await this.getStrings(content.text);
+        footerText.appendChild(
+          RemoteL10n.createElement(this.window.document, "span", {
+            content: content.text,
+          })
+        );
         options = { popupIconURL: content.addon.icon };
 
         footerLink.value = await this.getStrings({
           string_id: "cfr-doorhanger-extension-learn-more-link",
         });
         footerLink.setAttribute("href", content.addon.amo_url);
         footerLink.onclick = () =>
           this._sendTelemetry({
@@ -1018,56 +1045,16 @@ const CFRPageActions = {
       Cu.reportError(
         "Failed to get the latest add-on version for this recommendation"
       );
     }
     return url;
   },
 
   /**
-   * Show Milestone notification.
-   * @param browser                 The browser for the recommendation
-   * @param recommendation          The recommendation to show
-   * @param dispatchToASRouter      A function to dispatch resulting actions to
-   * @return                        Did adding the recommendation succeed?
-   */
-  async showMilestone(browser, message, dispatchToASRouter, options = {}) {
-    let win = null;
-    const { id, content, personalizedModelVersion } = message;
-
-    // If we are forcing via the Admin page, the browser comes in a different format
-    if (options.force) {
-      win = browser.browser.ownerGlobal;
-      RecommendationMap.set(browser.browser, {
-        id,
-        content,
-        retain: true,
-        modelVersion: personalizedModelVersion,
-      });
-    } else {
-      win = browser.ownerGlobal;
-      RecommendationMap.set(browser, {
-        id,
-        content,
-        retain: true,
-        modelVersion: personalizedModelVersion,
-      });
-    }
-
-    if (!PageActionMap.has(win)) {
-      PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
-    }
-
-    await PageActionMap.get(win).showMilestonePopup();
-    PageActionMap.get(win).addImpression(message);
-
-    return true;
-  },
-
-  /**
    * Force a recommendation to be shown. Should only happen via the Admin page.
    * @param browser                 The browser for the recommendation
    * @param recommendation  The recommendation to show
    * @param dispatchToASRouter      A function to dispatch resulting actions to
    * @return                        Did adding the recommendation succeed?
    */
   async forceRecommendation(browser, recommendation, dispatchToASRouter) {
     // If we are forcing via the Admin page, the browser comes in a different format
@@ -1079,18 +1066,23 @@ const CFRPageActions = {
       retain: true,
       modelVersion: personalizedModelVersion,
     });
     if (!PageActionMap.has(win)) {
       PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
     }
 
     if (content.skip_address_bar_notifier) {
-      await PageActionMap.get(win).showPopup();
-      PageActionMap.get(win).addImpression(recommendation);
+      if (recommendation.template === "milestone_message") {
+        await PageActionMap.get(win).showMilestonePopup();
+        PageActionMap.get(win).addImpression(recommendation);
+      } else {
+        await PageActionMap.get(win).showPopup();
+        PageActionMap.get(win).addImpression(recommendation);
+      }
     } else {
       await PageActionMap.get(win).showAddressBarNotifier(recommendation, true);
     }
     return true;
   },
 
   /**
    * Add a recommendation specific to the given browser and host.
@@ -1124,19 +1116,26 @@ const CFRPageActions = {
       retain: true,
       modelVersion: personalizedModelVersion,
     });
     if (!PageActionMap.has(win)) {
       PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
     }
 
     if (content.skip_address_bar_notifier) {
-      await PageActionMap.get(win).showPopup();
-      PageActionMap.get(win).addImpression(recommendation);
+      if (recommendation.template === "milestone_message") {
+        await PageActionMap.get(win).showMilestonePopup();
+        PageActionMap.get(win).addImpression(recommendation);
+      } else {
+        // Tracking protection messages
+        await PageActionMap.get(win).showPopup();
+        PageActionMap.get(win).addImpression(recommendation);
+      }
     } else {
+      // Doorhanger messages
       await PageActionMap.get(win).showAddressBarNotifier(recommendation, true);
     }
     return true;
   },
 
   /**
    * Clear all recommendations and hide all PageActions
    */
--- a/browser/components/newtab/lib/RemoteL10n.jsm
+++ b/browser/components/newtab/lib/RemoteL10n.jsm
@@ -22,16 +22,44 @@ XPCOMUtils.defineLazyModuleGetters(this,
   Services: "resource://gre/modules/Services.jsm",
 });
 
 class _RemoteL10n {
   constructor() {
     this._l10n = null;
   }
 
+  createElement(doc, elem, options = {}) {
+    let node;
+    if (options.content && options.content.string_id) {
+      node = doc.createElement("remote-text");
+    } else {
+      node = doc.createElementNS("http://www.w3.org/1999/xhtml", elem);
+    }
+    if (options.classList) {
+      node.classList.add(options.classList);
+    }
+    this.setString(node, options);
+
+    return node;
+  }
+
+  // If `string_id` is present it means we are relying on fluent for translations.
+  // Otherwise, we have a vanilla string.
+  setString(el, { content, attributes = {} }) {
+    if (content && content.string_id) {
+      for (let [fluentId, value] of Object.entries(attributes)) {
+        el.setAttribute(`fluent-variable-${fluentId}`, value);
+      }
+      el.setAttribute("fluent-remote-id", content.string_id);
+    } else {
+      el.textContent = content;
+    }
+  }
+
   /**
    * Creates a new DOMLocalization instance with the Fluent file from Remote Settings.
    *
    * Note: it will use the local Fluent file in any of following cases:
    *   * the remote Fluent file is not available
    *   * it was told to use the local Fluent file
    */
   _createDOML10n() {
--- a/browser/components/newtab/lib/ToolbarPanelHub.jsm
+++ b/browser/components/newtab/lib/ToolbarPanelHub.jsm
@@ -8,16 +8,17 @@ const { XPCOMUtils } = ChromeUtils.impor
 );
 XPCOMUtils.defineLazyModuleGetters(this, {
   Services: "resource://gre/modules/Services.jsm",
   EveryWindow: "resource:///modules/EveryWindow.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
   Preferences: "resource://gre/modules/Preferences.jsm",
   SpecialMessageActions:
     "resource://messaging-system/lib/SpecialMessageActions.jsm",
+  RemoteL10n: "resource://activity-stream/lib/RemoteL10n.jsm",
 });
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "TrackingDBService",
   "@mozilla.org/tracking-db-service;1",
   "nsITrackingDBService"
 );
 
@@ -262,58 +263,58 @@ class _ToolbarPanelHub {
       if (e.key === "Enter" || e.key === " ") {
         this._dispatchUserAction(win, message);
       }
     });
   }
 
   _createMessageElements(win, doc, message, previousDate) {
     const { content } = message;
-    const messageEl = this._createElement(doc, "div");
+    const messageEl = RemoteL10n.createElement(doc, "div");
     messageEl.classList.add("whatsNew-message");
 
     // Only render date if it is different from the one rendered before.
     if (content.published_date !== previousDate) {
       messageEl.appendChild(
-        this._createElement(doc, "p", {
+        RemoteL10n.createElement(doc, "p", {
           classList: "whatsNew-message-date",
           content: new Date(content.published_date).toLocaleDateString(
             "default",
             {
               month: "long",
               day: "numeric",
               year: "numeric",
             }
           ),
         })
       );
     }
 
-    const wrapperEl = this._createElement(doc, "button");
+    const wrapperEl = RemoteL10n.createElement(doc, "button");
     wrapperEl.doCommand = () => this._dispatchUserAction(win, message);
     wrapperEl.classList.add("whatsNew-message-body");
     messageEl.appendChild(wrapperEl);
 
     if (content.icon_url) {
       wrapperEl.classList.add("has-icon");
-      const iconEl = this._createElement(doc, "img");
+      const iconEl = RemoteL10n.createElement(doc, "img");
       iconEl.src = content.icon_url;
       iconEl.classList.add("whatsNew-message-icon");
       if (content.icon_alt && content.icon_alt.string_id) {
         doc.l10n.setAttributes(iconEl, content.icon_alt.string_id);
       } else {
         iconEl.setAttribute("alt", content.icon_alt);
       }
       wrapperEl.appendChild(iconEl);
     }
 
     wrapperEl.appendChild(this._createMessageContent(win, doc, content));
 
     if (content.link_text) {
-      const anchorEl = this._createElement(doc, "a", {
+      const anchorEl = RemoteL10n.createElement(doc, "a", {
         classList: "text-link",
         content: content.link_text,
       });
       anchorEl.doCommand = () => this._dispatchUserAction(win, message);
       wrapperEl.appendChild(anchorEl);
     }
 
     // Attach event listener on entire message container
@@ -324,104 +325,93 @@ class _ToolbarPanelHub {
 
   /**
    * Return message title (optional subtitle) and body
    */
   _createMessageContent(win, doc, content) {
     const wrapperEl = new win.DocumentFragment();
 
     wrapperEl.appendChild(
-      this._createElement(doc, "h2", {
+      RemoteL10n.createElement(doc, "h2", {
         classList: "whatsNew-message-title",
         content: content.title,
+        attributes: this.state.contentArguments,
       })
     );
 
     switch (content.layout) {
       case "tracking-protections":
         wrapperEl.appendChild(
-          this._createElement(doc, "h4", {
+          RemoteL10n.createElement(doc, "h4", {
             classList: "whatsNew-message-subtitle",
             content: content.subtitle,
+            attributes: this.state.contentArguments,
           })
         );
         wrapperEl.appendChild(
-          this._createElement(doc, "h2", {
+          RemoteL10n.createElement(doc, "h2", {
             classList: "whatsNew-message-title-large",
             content: this.state.contentArguments[
               content.layout_title_content_variable
             ],
+            attributes: this.state.contentArguments,
           })
         );
         break;
     }
 
     wrapperEl.appendChild(
-      this._createElement(doc, "p", {
+      RemoteL10n.createElement(doc, "p", {
         content: content.body,
         classList: "whatsNew-message-content",
+        attributes: this.state.contentArguments,
       })
     );
 
     return wrapperEl;
   }
 
   _createHeroElement(win, doc, message) {
     this.maybeLoadCustomElement(win);
 
-    const messageEl = this._createElement(doc, "div");
+    const messageEl = RemoteL10n.createElement(doc, "div");
     messageEl.setAttribute("id", "protections-popup-message");
     messageEl.classList.add("whatsNew-hero-message");
-    const wrapperEl = this._createElement(doc, "div");
+    const wrapperEl = RemoteL10n.createElement(doc, "div");
     wrapperEl.classList.add("whatsNew-message-body");
     messageEl.appendChild(wrapperEl);
 
     wrapperEl.appendChild(
-      this._createElement(doc, "h2", {
+      RemoteL10n.createElement(doc, "h2", {
         classList: "whatsNew-message-title",
         content: message.content.title,
       })
     );
     wrapperEl.appendChild(
-      this._createElement(doc, "p", {
+      RemoteL10n.createElement(doc, "p", {
         classList: "protections-popup-content",
         content: message.content.body,
       })
     );
 
     if (message.content.link_text) {
-      let linkEl = this._createElement(doc, "a", {
+      let linkEl = RemoteL10n.createElement(doc, "a", {
         classList: "text-link",
         content: message.content.link_text,
       });
       linkEl.disabled = true;
       wrapperEl.appendChild(linkEl);
       this._attachCommandListener(win, linkEl, message);
     } else {
       this._attachCommandListener(win, wrapperEl, message);
     }
 
     return messageEl;
   }
 
-  _createElement(doc, elem, options = {}) {
-    let node;
-    if (options.content && options.content.string_id) {
-      node = doc.createElement("remote-text");
-    } else {
-      node = doc.createElementNS("http://www.w3.org/1999/xhtml", elem);
-    }
-    if (options.classList) {
-      node.classList.add(options.classList);
-    }
-    this._setString(node, options.content);
-
-    return node;
-  }
-
   async _contentArguments() {
     const { defaultEngine } = Services.search;
     // Between now and 6 weeks ago
     const dateTo = new Date();
     const dateFrom = new Date(dateTo.getTime() - 42 * 24 * 60 * 60 * 1000);
     const eventsByDate = await TrackingDBService.getEventsByDateRange(
       dateFrom,
       dateTo
@@ -453,31 +443,16 @@ class _ToolbarPanelHub {
       ...totalEvents,
       // Passing in `undefined` as string for the Fluent variable name
       // in order to match and select the message that does not require
       // the variable.
       searchEngineName: defaultEngine ? defaultEngine.name : "undefined",
     };
   }
 
-  // If `string_id` is present it means we are relying on fluent for translations.
-  // Otherwise, we have a vanilla string.
-  _setString(el, stringObj) {
-    if (stringObj && stringObj.string_id) {
-      for (let [fluentId, value] of Object.entries(
-        this.state.contentArguments || {}
-      )) {
-        el.setAttribute(`fluent-variable-${fluentId}`, value);
-      }
-      el.setAttribute("fluent-remote-id", stringObj.string_id);
-    } else {
-      el.textContent = stringObj;
-    }
-  }
-
   async _showAppmenuButton(win) {
     this.maybeInsertFTL(win);
     await this._showElement(
       win.browser.ownerDocument,
       APPMENU_BUTTON_ID,
       BUTTON_STRING_ID
     );
   }
--- a/browser/components/newtab/test/browser/browser_asrouter_cfr.js
+++ b/browser/components/newtab/test/browser/browser_asrouter_cfr.js
@@ -190,24 +190,16 @@ function trigger_cfr_panel(
   }
   if (use_single_secondary_button) {
     recommendation.content.buttons.secondary = [
       recommendation.content.buttons.secondary[0],
     ];
   }
 
   clearNotifications();
-  if (recommendation.template === "milestone_message") {
-    return CFRPageActions.showMilestone(
-      browser,
-      recommendation,
-      // Use the real AS dispatch method to trigger real notifications
-      ASRouter.dispatch
-    );
-  }
   return CFRPageActions.addRecommendation(
     browser,
     trigger,
     recommendation,
     // Use the real AS dispatch method to trigger real notifications
     ASRouter.dispatch
   );
 }