Bug 1570631 - Part 1: refactoring; r=k88hudson,andreio
authorLiang-Heng Chen <xeonchen@gmail.com>
Wed, 11 Sep 2019 08:53:50 +0000
changeset 492640 4ad27f27eb865d75bc5d50fb81cc647e092a0d7a
parent 492639 ba563f823a564dfade0327d003143d765ac9ab59
child 492641 5e87be03a7a1b56e42b032b5e1e32f4e274c86f3
push id36563
push usercbrindusan@mozilla.com
push dateWed, 11 Sep 2019 21:53:06 +0000
treeherdermozilla-central@26711f10f5f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk88hudson, andreio
bugs1570631
milestone71.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 1570631 - Part 1: refactoring; r=k88hudson,andreio 1. extract generation of `secondaryActions` to make one-secondary-button possible 2. exteact `addImpression` 3. extract `showPopup` Differential Revision: https://phabricator.services.mozilla.com/D43606
browser/components/newtab/lib/CFRPageActions.jsm
browser/components/newtab/test/unit/asrouter/constants.js
--- a/browser/components/newtab/lib/CFRPageActions.jsm
+++ b/browser/components/newtab/lib/CFRPageActions.jsm
@@ -74,16 +74,31 @@ class PageAction {
       "browser/branding/sync-brand.ftl",
       "branding/brand.ftl",
     ]);
 
     // Saved timeout IDs for scheduled state changes, so they can be cancelled
     this.stateTransitionTimeoutIDs = [];
   }
 
+  addImpression(recommendation) {
+    this._dispatchImpression(recommendation);
+    // Only send an impression ping upon the first expansion.
+    // Note that when the user clicks on the "show" button on the asrouter admin
+    // page (both `bucket_id` and `id` will be set as null), we don't want to send
+    // the impression ping in that case.
+    if (!!recommendation.id && !!recommendation.content.bucket_id) {
+      this._sendTelemetry({
+        message_id: recommendation.id,
+        bucket_id: recommendation.content.bucket_id,
+        event: "IMPRESSION",
+      });
+    }
+  }
+
   async showAddressBarNotifier(recommendation, shouldExpand = false) {
     this.container.hidden = false;
 
     this.label.value = await this.getStrings(
       recommendation.content.notification_text
     );
 
     this.button.setAttribute(
@@ -109,28 +124,17 @@ class PageAction {
     this.urlbar.addEventListener("focus", this._collapse);
 
     if (shouldExpand) {
       this._clearScheduledStateChanges();
 
       // After one second, expand
       this._expand(DELAY_BEFORE_EXPAND_MS);
 
-      this._dispatchImpression(recommendation);
-      // Only send an impression ping upon the first expansion.
-      // Note that when the user clicks on the "show" button on the asrouter admin
-      // page (both `bucket_id` and `id` will be set as null), we don't want to send
-      // the impression ping in that case.
-      if (!!recommendation.id && !!recommendation.content.bucket_id) {
-        this._sendTelemetry({
-          message_id: recommendation.id,
-          bucket_id: recommendation.content.bucket_id,
-          event: "IMPRESSION",
-        });
-      }
+      this.addImpression(recommendation);
     }
   }
 
   hideAddressBarNotifier() {
     this.container.hidden = true;
     this._clearScheduledStateChanges();
     this.urlbar.removeAttribute("cfr-recommendation-state");
     this.container.removeEventListener("click", this._showPopupOnClick);
@@ -574,62 +578,51 @@ class PageAction {
 
     const primaryBtnStrings = await this.getStrings(primary.label);
     const mainAction = {
       label: primaryBtnStrings,
       accessKey: primaryBtnStrings.attributes.accesskey,
       callback: primaryActionCallback,
     };
 
-    // For each secondary action, get the strings and attributes
-    const secondaryBtnStrings = [];
-    for (let button of secondary) {
+    let _renderSecondaryButtonAction = async (event, button) => {
       let label = await this.getStrings(button.label);
-      secondaryBtnStrings.push({ label, attributes: label.attributes });
-    }
-    const secondaryActions = [
-      {
-        label: secondaryBtnStrings[0].label,
-        accessKey: secondaryBtnStrings[0].attributes.accesskey,
+      let { attributes } = label;
+
+      return {
+        label,
+        accessKey: attributes.accesskey,
         callback: () => {
-          this.dispatchUserAction(secondary[0].action);
+          if (button.action) {
+            this.dispatchUserAction(button.action);
+          } else {
+            this._blockMessage(id);
+            this.hideAddressBarNotifier();
+            RecommendationMap.delete(browser);
+          }
+
           this._sendTelemetry({
             message_id: id,
             bucket_id: content.bucket_id,
-            event: "DISMISS",
+            event,
           });
         },
-      },
-      {
-        label: secondaryBtnStrings[1].label,
-        accessKey: secondaryBtnStrings[1].attributes.accesskey,
-        callback: () => {
-          this._blockMessage(id);
-          this.hideAddressBarNotifier();
-          this._sendTelemetry({
-            message_id: id,
-            bucket_id: content.bucket_id,
-            event: "BLOCK",
-          });
-          RecommendationMap.delete(browser);
-        },
-      },
-      {
-        label: secondaryBtnStrings[2].label,
-        accessKey: secondaryBtnStrings[2].attributes.accesskey,
-        callback: () => {
-          this.dispatchUserAction(secondary[2].action);
-          this._sendTelemetry({
-            message_id: id,
-            bucket_id: content.bucket_id,
-            event: "MANAGE",
-          });
-        },
-      },
-    ];
+      };
+    };
+
+    // For each secondary action, define default telemetry event
+    const defaultSecondaryEvent = ["DISMISS", "BLOCK", "MANAGE"];
+    const secondaryActions = await Promise.all(
+      secondary.map((button, i) => {
+        return _renderSecondaryButtonAction(
+          button.event || defaultSecondaryEvent[i],
+          button
+        );
+      })
+    );
 
     // Actually show the notification
     this.currentNotification = this.window.PopupNotifications.show(
       browser,
       POPUP_NOTIFICATION_ID,
       panelTitle,
       "cfr",
       mainAction,
@@ -650,22 +643,29 @@ class PageAction {
     const browser = this.window.gBrowser.selectedBrowser;
     if (!RecommendationMap.has(browser)) {
       // There's no recommendation for this browser, so the user shouldn't have
       // been able to click
       this.hideAddressBarNotifier();
       return;
     }
     const message = RecommendationMap.get(browser);
-    const { id, content } = message;
 
     // The recommendation should remain either collapsed or expanded while the
     // doorhanger is showing
     this._clearScheduledStateChanges(browser, message);
 
+    await this.showPopup();
+  }
+
+  async showPopup() {
+    const browser = this.window.gBrowser.selectedBrowser;
+    const message = RecommendationMap.get(browser);
+    const { id, content } = message;
+
     // A hacky way of setting the popup anchor outside the usual url bar icon box
     // See https://searchfox.org/mozilla-central/rev/847b64cc28b74b44c379f9bff4f415b97da1c6d7/toolkit/modules/PopupNotifications.jsm#42
     browser.cfrpopupnotificationanchor = this.container;
 
     this._sendTelemetry({
       message_id: id,
       bucket_id: content.bucket_id,
       event: "CLICK_DOORHANGER",
--- a/browser/components/newtab/test/unit/asrouter/constants.js
+++ b/browser/components/newtab/test/unit/asrouter/constants.js
@@ -115,17 +115,16 @@ export const FAKE_RECOMMENDATION = {
       },
       secondary: [
         {
           label: { string_id: "secondary_button_id" },
           action: { id: "secondary_action" },
         },
         {
           label: { string_id: "secondary_button_id_2" },
-          action: { id: "secondary_action" },
         },
         {
           label: { string_id: "secondary_button_id_3" },
           action: { id: "secondary_action" },
         },
       ],
     },
   },