Bug 1009266 - Make about:newtab use 2x-DPI logos in 1x-DPI windows when a 1x logo isn't available. r=ttaubert

browser/base/content/newtab/search.js

@@ -146,7 +146,7 @@ let gSearch = {
 
     // Set the logo.
     let logoURI = window.devicePixelRatio == 2 ? engine.logo2xURI :
-                  engine.logoURI;
+                  engine.logoURI || engine.logo2xURI;
     if (logoURI) {
       this._nodes.logo.hidden = false;
       this._nodes.logo.style.backgroundImage = "url(" + logoURI + ")";

browser/base/content/test/newtab/browser.ini

@@ -2,8 +2,6 @@
 skip-if = e10s # Bug ?????? - about:newtab tests don't work in e10s
 support-files =
   head.js
-  searchEngineLogo.xml
-  searchEngineNoLogo.xml
 
 [browser_newtab_background_captures.js]
 [browser_newtab_block.js]
@@ -30,6 +28,11 @@ skip-if = os == "mac" # Intermittent failures, bug 898317
 [browser_newtab_perwindow_private_browsing.js]
 [browser_newtab_reset.js]
 [browser_newtab_search.js]
+support-files =
+  searchEngineNoLogo.xml
+  searchEngine1xLogo.xml
+  searchEngine2xLogo.xml
+  searchEngine1x2xLogo.xml
 [browser_newtab_sponsored_icon_click.js]
 [browser_newtab_tabsync.js]
 [browser_newtab_undo.js]

browser/base/content/test/newtab/browser_newtab_search.js

@@ -4,13 +4,15 @@
 // See browser/components/search/test/browser_*_behavior.js for tests of actual
 // searches.
 
-const ENGINE_LOGO = "searchEngineLogo.xml";
 const ENGINE_NO_LOGO = "searchEngineNoLogo.xml";
+const ENGINE_1X_LOGO = "searchEngine1xLogo.xml";
+const ENGINE_2X_LOGO = "searchEngine2xLogo.xml";
+const ENGINE_1X_2X_LOGO = "searchEngine1x2xLogo.xml";
 
 const SERVICE_EVENT_NAME = "ContentSearchService";
 
-const LOGO_LOW_DPI_SIZE = [65, 26];
-const LOGO_HIGH_DPI_SIZE = [130, 52];
+const LOGO_1X_DPI_SIZE = [65, 26];
+const LOGO_2X_DPI_SIZE = [130, 52];
 
 // The test has an expected search event queue and a search event listener.
 // Search events that are expected to happen are added to the queue, and the
@@ -42,31 +44,61 @@ function runTests() {
   // children, which makes the test click the wrong children, so disable it.
   panel.setAttribute("animate", "false");
 
-  // Add the two test engines.
-  let logoEngine = null;
-  yield promiseNewSearchEngine(true).then(engine => {
-    logoEngine = engine;
-    TestRunner.next();
-  });
-  ok(!!logoEngine.getIconURLBySize(...LOGO_LOW_DPI_SIZE),
-     "Sanity check: engine should have 1x logo");
-  ok(!!logoEngine.getIconURLBySize(...LOGO_HIGH_DPI_SIZE),
-     "Sanity check: engine should have 2x logo");
-
+  // Add the engine without any logos and switch to it.
   let noLogoEngine = null;
-  yield promiseNewSearchEngine(false).then(engine => {
+  yield promiseNewSearchEngine(ENGINE_NO_LOGO, 0).then(engine => {
     noLogoEngine = engine;
     TestRunner.next();
   });
-  ok(!noLogoEngine.getIconURLBySize(...LOGO_LOW_DPI_SIZE),
+  ok(!noLogoEngine.getIconURLBySize(...LOGO_1X_DPI_SIZE),
      "Sanity check: engine should not have 1x logo");
-  ok(!noLogoEngine.getIconURLBySize(...LOGO_HIGH_DPI_SIZE),
+  ok(!noLogoEngine.getIconURLBySize(...LOGO_2X_DPI_SIZE),
      "Sanity check: engine should not have 2x logo");
-
-  // Use the search service to change the current engine to the logo engine.
-  Services.search.currentEngine = logoEngine;
+  Services.search.currentEngine = noLogoEngine;
   yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);
-  checkCurrentEngine(ENGINE_LOGO);
+  checkCurrentEngine(ENGINE_NO_LOGO, false, false);
+
+  // Add the engine with a 1x-DPI logo and switch to it.
+  let logo1xEngine = null;
+  yield promiseNewSearchEngine(ENGINE_1X_LOGO, 1).then(engine => {
+    logo1xEngine = engine;
+    TestRunner.next();
+  });
+  ok(!!logo1xEngine.getIconURLBySize(...LOGO_1X_DPI_SIZE),
+     "Sanity check: engine should have 1x logo");
+  ok(!logo1xEngine.getIconURLBySize(...LOGO_2X_DPI_SIZE),
+     "Sanity check: engine should not have 2x logo");
+  Services.search.currentEngine = logo1xEngine;
+  yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);
+  checkCurrentEngine(ENGINE_1X_LOGO, true, false);
+
+  // Add the engine with a 2x-DPI logo and switch to it.
+  let logo2xEngine = null;
+  yield promiseNewSearchEngine(ENGINE_2X_LOGO, 1).then(engine => {
+    logo2xEngine = engine;
+    TestRunner.next();
+  });
+  ok(!logo2xEngine.getIconURLBySize(...LOGO_1X_DPI_SIZE),
+     "Sanity check: engine should not have 1x logo");
+  ok(!!logo2xEngine.getIconURLBySize(...LOGO_2X_DPI_SIZE),
+     "Sanity check: engine should have 2x logo");
+  Services.search.currentEngine = logo2xEngine;
+  yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);
+  checkCurrentEngine(ENGINE_2X_LOGO, false, true);
+
+  // Add the engine with 1x- and 2x-DPI logos and switch to it.
+  let logo1x2xEngine = null;
+  yield promiseNewSearchEngine(ENGINE_1X_2X_LOGO, 2).then(engine => {
+    logo1x2xEngine = engine;
+    TestRunner.next();
+  });
+  ok(!!logo1x2xEngine.getIconURLBySize(...LOGO_1X_DPI_SIZE),
+     "Sanity check: engine should have 1x logo");
+  ok(!!logo1x2xEngine.getIconURLBySize(...LOGO_2X_DPI_SIZE),
+     "Sanity check: engine should have 2x logo");
+  Services.search.currentEngine = logo1x2xEngine;
+  yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);
+  checkCurrentEngine(ENGINE_1X_2X_LOGO, true, true);
 
   // Click the logo to open the search panel.
   yield Promise.all([
@@ -89,12 +121,12 @@ function runTests() {
     promiseClick(noLogoBox),
   ]).then(TestRunner.next);
 
-  checkCurrentEngine(ENGINE_NO_LOGO);
+  checkCurrentEngine(ENGINE_NO_LOGO, false, false);
 
-  // Switch back to the logo engine.
-  Services.search.currentEngine = logoEngine;
+  // Switch back to the 1x-and-2x logo engine.
+  Services.search.currentEngine = logo1x2xEngine;
   yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);
-  checkCurrentEngine(ENGINE_LOGO);
+  checkCurrentEngine(ENGINE_1X_2X_LOGO, true, true);
 
   // Open the panel again.
   yield Promise.all([
@@ -157,16 +189,15 @@ function promiseSearchEvents(events) {
   return Promise.all(events.map(e => e.deferred.promise));
 }
 
-function promiseNewSearchEngine(withLogo) {
-  let basename = withLogo ? ENGINE_LOGO : ENGINE_NO_LOGO;
+function promiseNewSearchEngine(basename, numLogos) {
   info("Waiting for engine to be added: " + basename);
 
   // Wait for the search events triggered by adding the new engine.
   // engine-added engine-loaded
   let expectedSearchEvents = ["State", "State"];
-  if (withLogo) {
-    // an engine-changed for each of the two logos
-    expectedSearchEvents.push("State", "State");
+  // engine-changed for each of the logos
+  for (let i = 0; i < numLogos; i++) {
+    expectedSearchEvents.push("State");
   }
   let eventPromise = promiseSearchEvents(expectedSearchEvents);
 
@@ -197,7 +228,7 @@ function promiseNewSearchEngine(withLogo) {
   return deferred.promise;
 }
 
-function checkCurrentEngine(basename) {
+function checkCurrentEngine(basename, has1xLogo, has2xLogo) {
   let engine = Services.search.currentEngine;
   ok(engine.name.contains(basename),
      "Sanity check: current engine: engine.name=" + engine.name +
@@ -208,8 +239,23 @@ function checkCurrentEngine(basename) {
      "currentEngineName: " + engine.name);
 
   // search bar logo
-  let logoSize = [px * window.devicePixelRatio for (px of LOGO_LOW_DPI_SIZE)];
-  let logoURI = engine.getIconURLBySize(...logoSize);
+  let logoURI = null;
+  if (window.devicePixelRatio == 2) {
+    if (has2xLogo) {
+      logoURI = engine.getIconURLBySize(...LOGO_2X_DPI_SIZE);
+      ok(!!logoURI, "Sanity check: engine should have 2x logo");
+    }
+  }
+  else {
+    if (has1xLogo) {
+      logoURI = engine.getIconURLBySize(...LOGO_1X_DPI_SIZE);
+      ok(!!logoURI, "Sanity check: engine should have 1x logo");
+    }
+    else if (has2xLogo) {
+      logoURI = engine.getIconURLBySize(...LOGO_2X_DPI_SIZE);
+      ok(!!logoURI, "Sanity check: engine should have 2x logo");
+    }
+  }
   let logo = logoImg();
   is(logo.hidden, !logoURI,
      "Logo should be visible iff engine has a logo: " + engine.name);