Show better info when no 3rd party resources present.

src/js/background.js

@@ -494,7 +494,11 @@ Badger.prototype = {
    * @returns {*} A dictionary of third party origins and their actions
    */
   getAllOriginsForTab: function(tabId) {
-    return Object.keys(this.tabData[tabId].origins);
+    try {
+      return Object.keys(this.tabData[tabId].origins);
+    } catch (e) { // No tabData, tab, or origins.
+      return [];
+    }
   },
 
   /**

src/js/firefoxandroid.js

@@ -22,6 +22,9 @@
  */
 
 require.scopes.firefoxandroid = (function() {
+
+var utils = require('utils');
+
 var isFirefoxAndroid = !(
   chrome.browserAction.setBadgeText &&
   chrome.browserAction.setPopup &&
@@ -35,10 +38,8 @@ var popup_url = chrome.runtime.getManifest().browser_action.default_popup;
 // fakes a popup
 function openPopup() {
   chrome.tabs.query({active: true, lastFocusedWindow: true}, (tabs) => {
-    var url = popup_url + "?tabId=" + tabs[0].id;
-    chrome.tabs.create({url, index: tabs[0].index + 1}, (tab) => {
-      openPopupId = tab.id;
-    });
+    utils.openPopupForTab(tabs[0])
+      .then(popupTab => openPopupId = popupTab.id);
   });
 }
 
@@ -71,19 +72,10 @@ function startListeners() {
   }
 }
 
-// Used in popup.js, figures out which tab opened the 'fake' popup
-function getParentOfPopup(callback){
-  chrome.tabs.query({active: true, lastFocusedWindow: true}, function(focusedTab) {
-    var parentId = parseInt(new URL(focusedTab[0].url).searchParams.get('tabId'));
-    chrome.tabs.get(parentId, callback);
-  });
-}
-
 /************************************** exports */
 var exports = {};
 exports.startListeners = startListeners;
 exports.isUsed = isFirefoxAndroid;
-exports.getParentOfPopup = getParentOfPopup;
 return exports;
 /************************************** exports */
 })();

src/js/popup.js

@@ -22,7 +22,6 @@ var backgroundPage = chrome.extension.getBackgroundPage();
 var require = backgroundPage.require;
 var constants = backgroundPage.constants;
 var badger = backgroundPage.badger;
-var FirefoxAndroid = backgroundPage.FirefoxAndroid;
 var htmlUtils = require("htmlutils").htmlUtils;
 
 var i18n = chrome.i18n;
@@ -282,8 +281,9 @@ function refreshPopup(tabId) {
   //TODO this is calling get action and then being used to call get Action
   var origins = badger.getAllOriginsForTab(tabId);
   if (!origins || origins.length === 0) {
-    $("#blockedResources").html(i18n.getMessage("popup_blocked"));
-    $('#number_trackers').text('0');
+    $("#pbInstructions")
+      .html(i18n.getMessage("popup_blocked"))
+      .show();
     return;
   }
 
@@ -357,6 +357,7 @@ function refreshPopup(tabId) {
   }
 
   $('#number_trackers').text(trackerCount);
+  $('#pbInstructions').show();
 
   function renderDomains() {
     const CHUNK = 1;
@@ -513,13 +514,13 @@ function syncUISelections() {
 * seems to be that it is synchronous.
 */
 function getTab(callback) {
-  // Temporary fix for Firefox Android
-  if(FirefoxAndroid.isUsed){
-    FirefoxAndroid.getParentOfPopup(callback);
-    return;
+  // If opened with a tabId parameter, use that.
+  let tabId = parseInt(new URL(window.location.href).searchParams.get('tabId'));
+  if (!Number.isNaN(tabId)) {
+    return chrome.tabs.get(tabId, callback);
   }
 
-  chrome.tabs.query({active: true, lastFocusedWindow: true}, function(t) { callback(t[0]); });
+  chrome.tabs.query({active: true, lastFocusedWindow: true}, (t) => callback(t[0]));
 }
 
 document.addEventListener('DOMContentLoaded', function () {

src/js/utils.js

@@ -145,6 +145,14 @@ function estimateMaxEntropy(str) {
   return maxBits;  // May return Infinity when the content is too long.
 }
 
+function openPopupForTab(tab) {
+  var popup_url = chrome.runtime.getManifest().browser_action.default_popup,
+    url = popup_url + "?tabId=" + tab.id;
+  return new Promise(resolve => {
+    chrome.tabs.create({url, index: tab.index + 1}, resolve);
+  });
+}
+
 /**
  * Get a random number in the inclusive range min..max
  *
@@ -335,6 +343,7 @@ exports.oneDay = oneDay;
 exports.oneHour = oneHour;
 exports.oneMinute = oneMinute;
 exports.oneSecond = oneSecond;
+exports.openPopupForTab = openPopupForTab;
 exports.parseCookie = parseCookie;
 exports.rateLimit = rateLimit;
 exports.removeElementFromArray = removeElementFromArray;

src/skin/popup.html

@@ -70,7 +70,7 @@ <h1 id="report_title" class="i18n_report_title"></h1>
   <div id="subtitle"><span id="version" class="i18n_version"></span></div>
 </div>
 <div id="blockedResourcesContainer">
-  <p id="pbInstructions"> <span class="i18n_pb_detected"></span> <span id='number_trackers'></span> <span class="i18n_popup_instructions"></span></p>
+  <p id="pbInstructions" style="display:none"><span class="i18n_pb_detected"></span> <span id='number_trackers'></span> <span class="i18n_popup_instructions"></span></p>
   <div class="spacer"></div>
   <div id="blockedResources"><span class="options_loading"></span></div>
 </div>

tests/selenium/pbtest.py

@@ -290,6 +290,9 @@ def wait_for_script(self, script, timeout=SEL_DEFAULT_WAIT_TIMEOUT):
             "Timed out waiting for execute_script to eval to True"
         )
 
+    def wait(self, func, timeout=SEL_DEFAULT_WAIT_TIMEOUT):
+        return WebDriverWait(self.driver, timeout).until(func)
+
     @property
     def logs(self):
         def strip(l):

tests/selenium/popup_test.py

@@ -14,24 +14,57 @@
 class PopupTest(pbtest.PBSeleniumTest):
     """Make sure the popup works correctly."""
 
+    def open_url_and_popup(self, url=None, close_overlay=True):
+        js_src = """/**
+* open a page, wait, then open the popup for it
+*/
+(() => {
+chrome.tabs.create(%s, tab => {
+  setTimeout(() => utils.openPopupForTab(tab), 2000);
+});
+})();
+"""
+        if url is None:
+            js_src %= '{}'
+        else:
+            js_src %= ('{url: "%s"}' % url)
+
+        self.load_url(self.bg_url)
+
+        before_handles = len(self.driver.window_handles)
+        self.js(js_src)
+        while len(self.driver.window_handles) < before_handles + 2:
+            time.sleep(0.1)
+        for w in self.driver.window_handles:
+            self.driver.switch_to.window(w)
+            if 'popup.html' in self.driver.current_url:
+                break
+        if close_overlay:
+            self.close_popup_overlay(ignore_missing_overlay=True)
+
+    def close_popup_overlay(self, ignore_missing_overlay=False):
+        # Click 'X' element to close overlay.
+        try:
+            close_element = self.find_el_by_css('#fittslaw')
+        except (NoSuchElementException, TimeoutException) as e:
+            if ignore_missing_overlay:
+                return
+            self.fail("Unable to find element to close popup overlay")
+        close_element.click()
+
+        # Element will fade out so wait for it to disappear.
+        try:
+            WebDriverWait(self.driver, 5).until(
+                expected_conditions.invisibility_of_element_located(
+                    (By.ID, "fittslaw")))
+        except TimeoutException:
+            self.fail("Unable to close popup overlay")
+
     def open_popup(self, close_overlay=True):
         """Open popup and optionally close overlay."""
         self.load_url(self.popup_url, wait_on_site=1)
         if close_overlay:
-            # Click 'X' element to close overlay.
-            try:
-                close_element = self.driver.find_element_by_id("fittslaw")
-            except NoSuchElementException:
-                self.fail("Unable to find element to close popup overlay")
-            close_element.click()
-
-            # Element will fade out so wait for it to disappear.
-            try:
-                WebDriverWait(self.driver, 5).until(
-                    expected_conditions.invisibility_of_element_located(
-                        (By.ID, "fittslaw")))
-            except TimeoutException:
-                self.fail("Unable to close popup overlay")
+            self.close_popup_overlay()
 
     def get_enable_button(self):
         """Get enable button on popup."""
@@ -118,28 +151,31 @@ def test_options_button(self):
 
     def test_trackers_link(self):
         """Ensure trackers link opens EFF website."""
-        self.open_popup()
+        url = "https://cdn.rawgit.com/cowlicks/5d5c0c5b674268a74c62a8b1000ade69/raw/6893f945fd707c57cdfa7eef2d309f77fab92621/one_origin.html"
+        self.open_url_and_popup(url);
 
         try:
             trackers_link = self.driver.find_element_by_link_text("trackers")
         except NoSuchElementException:
             self.fail("Unable to find trackers link on popup")
 
-        handles_before = set(self.driver.window_handles)
+        handles_before = self.driver.window_handles
         trackers_link.click()
-
-        # Make sure EFF website not opened in same window.
-        time.sleep(5)
-        if self.driver.current_url != self.popup_url:
-            self.fail("EFF website not opened in new window")
+        self.wait(lambda driver: len(handles_before) != len(self.driver.window_handles))
+        new_handle = set(self.driver.window_handles).difference(set(handles_before)).pop()
+        self.driver.switch_to.window(new_handle)
 
         # Look for EFF website and return if found.
-        new_handle = set(self.driver.window_handles).difference(handles_before)
-        self.driver.switch_to.window(new_handle.pop())
-
         eff_url = "https://www.eff.org/privacybadger#faq-What-is-a-third-party-tracker?"
+        self.wait(lambda driver: driver.current_url == eff_url)
         self.assertEqual(self.driver.current_url, eff_url,
-            "EFF website should open after clicking donate button on popup")
+                "tracker explanation should open after clicking trackers button on popup")
+
+    def test_no_third_party(self):
+        self.open_url_and_popup()
+
+        self.assertIn('no third party', self.find_el_by_css('#pbInstructions').text)
+        self.assertFalse(self.find_el_by_css('#blockedResources').is_displayed())
 
     def test_disable_enable_buttons(self):
         """Ensure disable/enable buttons change popup state."""