Backed out changeset 3a362a5d8b87 (bug 1631415) for Newtab failure in activity-stream. CLOSED TREE
authorDorel Luca <dluca@mozilla.com>
Fri, 29 May 2020 22:36:58 +0300
changeset 533036 a58cc68b0c51645d34cbc7f8f8a30edae77ab396
parent 533035 8e79edbbce00f5ea1326017179de917f7137c12f
child 533037 094b82fbec62e99e55cea4c33804f1273f950546
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)
bugs1631415
milestone78.0a1
backs out3a362a5d8b877e7ed26cdd5e61a13b1ea00f8cb1
first release with
nightly linux32
a58cc68b0c51 / 78.0a1 / 20200529214631 / files
nightly linux64
a58cc68b0c51 / 78.0a1 / 20200529214631 / files
nightly mac
a58cc68b0c51 / 78.0a1 / 20200529214631 / files
nightly win32
a58cc68b0c51 / 78.0a1 / 20200529214631 / files
nightly win64
a58cc68b0c51 / 78.0a1 / 20200529214631 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 3a362a5d8b87 (bug 1631415) for Newtab failure in activity-stream. CLOSED TREE
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,24 +16,19 @@
 
       this._content = null;
     }
 
     get fluentAttributeValues() {
       const attributes = {};
       for (let name of this.getAttributeNames()) {
         if (name.startsWith("fluent-variable-")) {
-          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;
+          attributes[name.replace(/^fluent-variable-/, "")] = this.getAttribute(
+            name
+          );
         }
       }
 
       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,17 +1343,16 @@ 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
@@ -1378,16 +1377,19 @@ 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,25 +302,16 @@ 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.
    */
@@ -512,18 +503,16 @@ 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",
@@ -538,28 +527,31 @@ class PageAction {
     );
     let reachedMilestone = 0;
     let totalSaved = await TrackingDBService.sumAllEvents();
     for (let milestone of milestones) {
       if (totalSaved >= milestone) {
         reachedMilestone = milestone;
       }
     }
-    if (headerLabel.firstChild) {
-      headerLabel.firstChild.remove();
-    }
-    headerLabel.appendChild(
-      RemoteL10n.createElement(this.window.document, "span", {
-        content: message.content.heading_text,
-        attributes: {
+    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,
+        {
           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")
@@ -634,18 +626,16 @@ 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"
     );
@@ -688,24 +678,17 @@ 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"
         );
-        if (author.firstChild) {
-          author.firstChild.remove();
-        }
-        author.appendChild(
-          RemoteL10n.createElement(this.window.document, "span", {
-            content: content.text,
-          })
-        );
+        author.textContent = await this.getStrings(content.text);
         primaryActionCallback = () => {
           this._blockMessage(id);
           this.dispatchUserAction(primary.action);
           this.hideAddressBarNotifier();
           this._sendTelemetry({
             message_id: id,
             bucket_id: content.bucket_id,
             event: "ENABLE",
@@ -760,39 +743,29 @@ 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");
-            li.appendChild(
-              RemoteL10n.createElement(this.window.document, "span", {
-                content: step,
-              })
-            );
+            RemoteL10n.l10n.setAttributes(li, step.string_id);
             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.appendChild(
-          RemoteL10n.createElement(this.window.document, "span", {
-            content: content.text,
-          })
-        );
+        footerText.textContent = await this.getStrings(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({
@@ -1045,16 +1018,56 @@ 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
@@ -1066,23 +1079,18 @@ const CFRPageActions = {
       retain: true,
       modelVersion: personalizedModelVersion,
     });
     if (!PageActionMap.has(win)) {
       PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
     }
 
     if (content.skip_address_bar_notifier) {
-      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);
-      }
+      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.
@@ -1116,26 +1124,19 @@ const CFRPageActions = {
       retain: true,
       modelVersion: personalizedModelVersion,
     });
     if (!PageActionMap.has(win)) {
       PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
     }
 
     if (content.skip_address_bar_notifier) {
-      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);
-      }
+      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,44 +22,16 @@ 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,17 +8,16 @@ 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"
 );
 
@@ -263,58 +262,58 @@ class _ToolbarPanelHub {
       if (e.key === "Enter" || e.key === " ") {
         this._dispatchUserAction(win, message);
       }
     });
   }
 
   _createMessageElements(win, doc, message, previousDate) {
     const { content } = message;
-    const messageEl = RemoteL10n.createElement(doc, "div");
+    const messageEl = this._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(
-        RemoteL10n.createElement(doc, "p", {
+        this._createElement(doc, "p", {
           classList: "whatsNew-message-date",
           content: new Date(content.published_date).toLocaleDateString(
             "default",
             {
               month: "long",
               day: "numeric",
               year: "numeric",
             }
           ),
         })
       );
     }
 
-    const wrapperEl = RemoteL10n.createElement(doc, "button");
+    const wrapperEl = this._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 = RemoteL10n.createElement(doc, "img");
+      const iconEl = this._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 = RemoteL10n.createElement(doc, "a", {
+      const anchorEl = this._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
@@ -325,93 +324,104 @@ class _ToolbarPanelHub {
 
   /**
    * Return message title (optional subtitle) and body
    */
   _createMessageContent(win, doc, content) {
     const wrapperEl = new win.DocumentFragment();
 
     wrapperEl.appendChild(
-      RemoteL10n.createElement(doc, "h2", {
+      this._createElement(doc, "h2", {
         classList: "whatsNew-message-title",
         content: content.title,
-        attributes: this.state.contentArguments,
       })
     );
 
     switch (content.layout) {
       case "tracking-protections":
         wrapperEl.appendChild(
-          RemoteL10n.createElement(doc, "h4", {
+          this._createElement(doc, "h4", {
             classList: "whatsNew-message-subtitle",
             content: content.subtitle,
-            attributes: this.state.contentArguments,
           })
         );
         wrapperEl.appendChild(
-          RemoteL10n.createElement(doc, "h2", {
+          this._createElement(doc, "h2", {
             classList: "whatsNew-message-title-large",
             content: this.state.contentArguments[
               content.layout_title_content_variable
             ],
-            attributes: this.state.contentArguments,
           })
         );
         break;
     }
 
     wrapperEl.appendChild(
-      RemoteL10n.createElement(doc, "p", {
+      this._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 = RemoteL10n.createElement(doc, "div");
+    const messageEl = this._createElement(doc, "div");
     messageEl.setAttribute("id", "protections-popup-message");
     messageEl.classList.add("whatsNew-hero-message");
-    const wrapperEl = RemoteL10n.createElement(doc, "div");
+    const wrapperEl = this._createElement(doc, "div");
     wrapperEl.classList.add("whatsNew-message-body");
     messageEl.appendChild(wrapperEl);
 
     wrapperEl.appendChild(
-      RemoteL10n.createElement(doc, "h2", {
+      this._createElement(doc, "h2", {
         classList: "whatsNew-message-title",
         content: message.content.title,
       })
     );
     wrapperEl.appendChild(
-      RemoteL10n.createElement(doc, "p", {
+      this._createElement(doc, "p", {
         classList: "protections-popup-content",
         content: message.content.body,
       })
     );
 
     if (message.content.link_text) {
-      let linkEl = RemoteL10n.createElement(doc, "a", {
+      let linkEl = this._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
@@ -443,16 +453,31 @@ 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,16 +190,24 @@ 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
   );
 }