Fix #3574, Use download only mode if 'never remember history' is checked

mozilla-services · Oct 30, 2017 · d0d4db3 · d0d4db3
1 parent c6db3af
commit d0d4db3
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 11 deletions.
diff --git a/addon/bootstrap.js b/addon/bootstrap.js
@@ -5,6 +5,7 @@ const ADDON_ID = "screenshots@mozilla.org";
 const TELEMETRY_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
 const PREF_BRANCH = "extensions.screenshots.";
 const USER_DISABLE_PREF = "extensions.screenshots.disabled";
+const HISTORY_ENABLED_PREF = "places.history.enabled";
 
 const { interfaces: Ci, utils: Cu } = Components;
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -49,6 +50,21 @@ const prefObserver = {
   }
 };
 
+let historyEnabled = prefs.getBoolPref(HISTORY_ENABLED_PREF);
+// TODO: since checking "never remember history" requires a restart,
+// is it necessary to observe the places.history.enabled pref?
+const historyEnabledObserver = {
+  register() {
+    prefs.addObserver(HISTORY_ENABLED_PREF, this, false); // eslint-disable-line mozilla/no-useless-parameters
+  },
+  unregister() {
+    prefs.removeObserver(HISTORY_ENABLED_PREF, this, false); // eslint-disable-line mozilla/no-useless-parameters
+  },
+  observe(aSubject, aTopic, aData) {
+    historyEnabled = prefs.getBoolPref(HISTORY_ENABLED_PREF);
+  }
+};
+
 const appStartupObserver = {
   register() {
     Services.obs.addObserver(this, "sessionstore-windows-restored", false); // eslint-disable-line mozilla/no-useless-parameters
@@ -131,13 +147,15 @@ function startup(data, reason) { // eslint-disable-line no-unused-vars
     appStartupDone();
   }
   prefObserver.register();
+  historyEnabledObserver.register();
   addonResourceURI = data.resourceURI;
   // eslint-disable-next-line promise/catch-or-return
   appStartupPromise = appStartupPromise.then(handleStartup);
 }
 
 function shutdown(data, reason) { // eslint-disable-line no-unused-vars
   prefObserver.unregister();
+  historyEnabledObserver.unregister();
   const webExtension = LegacyExtensionsUtils.getEmbeddedExtensionFor({
     id: ADDON_ID,
     resourceURI: addonResourceURI
@@ -223,6 +241,9 @@ function handleMessage(msg, sender, sendReply) {
       sendReply({type: "success", value: !!addon});
     });
     return true;
+  } else if (msg.funcName === "getHistoryPref") {
+    let historyEnabled = getBoolPref(HISTORY_ENABLED_PREF);
+    sendReply({type: "success", value: historyEnabled});
   }
 }
 

diff --git a/addon/webextension/background/main.js b/addon/webextension/background/main.js
@@ -302,5 +302,11 @@ this.main = (function() {
     return startSelectionWithOnboarding(sender.tab);
   });
 
+  communication.register("isHistoryEnabled", () => {
+    return catcher.watchPromise(communication.sendToBootstrap("getHistoryPref").then(historyEnabled => {
+      return historyEnabled;
+    }));
+  });
+
   return exports;
 })();
diff --git a/addon/webextension/background/selectorLoader.js b/addon/webextension/background/selectorLoader.js
@@ -1,4 +1,4 @@
-/* globals catcher, log */
+/* globals catcher, communication, log */
 
 "use strict";
 
@@ -66,7 +66,7 @@ this.selectorLoader = (function() {
 
   exports.loadModules = function(tabId, hasSeenOnboarding) {
     loadingTabs.add(tabId);
-    let promise = incognitoCheck(tabId);
+    let promise = downloadOnlyCheck(tabId);
     if (hasSeenOnboarding) {
       promise = promise.then(() => {
         return executeModules(tabId, standardScripts.concat(selectorScripts));
@@ -85,14 +85,19 @@ this.selectorLoader = (function() {
     });
   };
 
-  function incognitoCheck(tabId) {
-    return browser.tabs.get(tabId).then(tab => {
-      return browser.tabs.executeScript(tabId, {
-        // Note: `window` here refers to a global accessible to content
-        // scripts, but not the scripts in the underlying page. For more
-        // details, see https://mdn.io/WebExtensions/Content_scripts#Content_script_environment
-        code: `window.downloadOnly = ${tab.incognito}`,
-        runAt: "document_start"
+  // TODO: since bootstrap communication is now required, would this function
+  // make more sense inside background/main?
+  function downloadOnlyCheck(tabId) {
+    return communication.sendToBootstrap("getHistoryPref").then((historyEnabled) => {
+      return browser.tabs.get(tabId).then(tab => {
+        let downloadOnly = !historyEnabled || tab.incognito;
+        return browser.tabs.executeScript(tabId, {
+          // Note: `window` here refers to a global accessible to content
+          // scripts, but not the scripts in the underlying page. For more
+          // details, see https://mdn.io/WebExtensions/Content_scripts#Content_script_environment
+          code: `window.downloadOnly = ${downloadOnly}`,
+          runAt: "document_start"
+        });
       });
     });
   }

diff --git a/addon/webextension/selector/ui.js b/addon/webextension/selector/ui.js
@@ -101,7 +101,9 @@ this.ui = (function() { // eslint-disable-line no-unused-vars
       <span data-l10n-id="downloadOnlyNotice"></span><span class="circle">?</span>
       <div class="download-only-details">
         <p data-l10n-id="downloadOnlyDetails">
-          <ul><li data-l10n-id="downloadOnlyDetailsPrivate">`;
+          <ul>
+            <li data-l10n-id="downloadOnlyDetailsPrivate"></li>
+            <li data-l10n-id="downloadOnlyDetailsNeverRemember"></li>`;
     localizeText(notice);
     return notice;
   }