Firefox peer feedback for 25.0.0 release

Changes: * use 1 message to send telemetry scalars * escape all reserved chars, not just quotes * remove needless thenable * simplify anyMatches * prefer Map to obj literal See also https://bugzilla.mozilla.org/show_bug.cgi?id=1419148#c13
mozilla-services · Nov 27, 2017 · fc7c3ef · fc7c3ef
1 parent 6bafd7d
commit fc7c3ef
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 18 deletions.
diff --git a/addon/bootstrap.js b/addon/bootstrap.js
@@ -228,15 +228,15 @@ function handleMessage(msg, sender, sendReply) {
   } else if (msg.funcName === "getHistoryPref") {
     let historyEnabled = getBoolPref(HISTORY_ENABLED_PREF);
     sendReply({type: "success", value: historyEnabled});
-  } else if (msg.funcName === "incrementDownloadCount") {
-    Services.telemetry.scalarAdd('screenshots.download', 1);
-    sendReply({type: "success", value: true});
-  } else if (msg.funcName === "incrementUploadCount") {
-    Services.telemetry.scalarAdd('screenshots.upload', 1);
-    sendReply({type: "success", value: true});
-  } else if (msg.funcName === "incrementCopyCount") {
-    Services.telemetry.scalarAdd('screenshots.copy', 1);
-    sendReply({type: "success", value: true});
+  } else if (msg.funcName === "incrementCount") {
+    let allowedScalars = ["download", "upload", "copy"];
+    let scalar = msg.args && msg.args[0] && msg.args[0].scalar;
+    if (!allowedScalars.includes(scalar)) {
+      sendReply({type: "error", name: `incrementCount passed an unrecognized scalar ${scalar}`});
+    } else {
+      Services.telemetry.scalarAdd(`screenshots.${scalar}`, 1);
+      sendReply({type: "success", value: true});
+    }
   }
 }
 

diff --git a/addon/webextension/background/analytics.js b/addon/webextension/background/analytics.js
@@ -113,7 +113,7 @@ this.analytics = (function() {
     return !!telemetryPref;
   };
 
-  let timingData = {};
+  let timingData = new Map();
 
   // Configuration for filtering the sendEvent stream on start/end events.
   // When start or end events occur, the time is recorded.
@@ -197,7 +197,7 @@ this.analytics = (function() {
   }
 
   function anyMatches(filters, action, label) {
-    return !!filters.find(filter => match(filter, action, label));
+    return filters.some(filter => match(filter, action, label));
   }
 
   function measureTiming(action, label) {

diff --git a/addon/webextension/background/main.js b/addon/webextension/background/main.js
@@ -209,7 +209,7 @@ this.main = (function() {
     return blobConverters.blobToArray(blob).then(buffer => {
       return browser.clipboard.setImageData(
         buffer, blob.type.split("/", 2)[1]).then(() => {
-          catcher.watchPromise(communication.sendToBootstrap('incrementCopyCount'));
+          catcher.watchPromise(communication.sendToBootstrap("incrementCount", {scalar: "copy"}));
           return browser.notifications.create({
             type: "basic",
             iconUrl: "../icons/copy.png",
@@ -236,13 +236,11 @@ this.main = (function() {
       }
     });
     browser.downloads.onChanged.addListener(onChangedCallback)
-    catcher.watchPromise(communication.sendToBootstrap("incrementDownloadCount"));
+    catcher.watchPromise(communication.sendToBootstrap("incrementCount", {scalar: "download"}));
     return browser.windows.getLastFocused().then(windowInfo => {
-      return windowInfo.incognito;
-    }).then((incognito) => {
       return browser.downloads.download({
         url,
-        incognito,
+        incognito: windowInfo.incognito,
         filename: info.filename
       }).then((id) => {
         downloadId = id;

diff --git a/addon/webextension/background/takeshot.js b/addon/webextension/background/takeshot.js
@@ -87,7 +87,7 @@ this.takeshot = (function() {
         }
       );
     }).then(() => {
-      catcher.watchPromise(communication.sendToBootstrap('incrementUploadCount'));
+      catcher.watchPromise(communication.sendToBootstrap("incrementCount", {scalar: "upload"}));
       return shot.viewUrl;
     }).catch((error) => {
       browser.tabs.remove(openedTab.id);

diff --git a/addon/webextension/selector/ui.js b/addon/webextension/selector/ui.js
@@ -386,7 +386,11 @@ this.ui = (function() { // eslint-disable-line no-unused-vars
 
   function getAttributeText(l10nID) {
     let text = browser.i18n.getMessage(l10nID);
-    return text && text.replace('"', "&quot;");
+    return text &&
+      text.replace("&", "&amp;")
+        .replace('"', "&quot;")
+        .replace("<", "&lt;")
+        .replace(">", "&gt;");
   }
 
   let iframePreview = exports.iframePreview = {