Bug 1516658: Favicon requests should use an accept header appropriate for images. r=Gijs
authorDave Townsend <dtownsend@oxymoronical.com>
Mon, 07 Jan 2019 17:43:23 +0000
changeset 509828 43c43fa35d2a9f4cd07548723cd3a9cdf66bf73d
parent 509827 f4fc0e5c9d4a8fcc6e93cff2cde0362ac04b8016
child 509829 cf3598ee865bd97dcbd36656e46617a5a2b9456d
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1516658
milestone66.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 1516658: Favicon requests should use an accept header appropriate for images. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D15778
browser/base/content/test/favicons/accept.html
browser/base/content/test/favicons/accept.sjs
browser/base/content/test/favicons/browser.ini
browser/base/content/test/favicons/browser_favicon_accept.js
browser/modules/FaviconLoader.jsm
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/favicons/accept.html
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test file for accept header</title>
+  <link rel="icon" href="accept.sjs">
+</head>
+<body>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/favicons/accept.sjs
@@ -0,0 +1,15 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+function handleRequest(request, response) {
+  // Doesn't seem any way to get the value from prefs from here. :(
+  let expected = "image/webp,*/*";
+  if (expected != request.getHeader("Accept")) {
+    response.setStatusLine(request.httpVersion, 404, "Not Found");
+    return;
+  }
+
+  response.setStatusLine(request.httpVersion, 302, "Moved Temporarily");
+  response.setHeader("Location", "moz.png");
+}
--- a/browser/base/content/test/favicons/browser.ini
+++ b/browser/base/content/test/favicons/browser.ini
@@ -65,8 +65,12 @@ support-files =
 [browser_favicon_cache.js]
 support-files =
   cookie_favicon.sjs
   cookie_favicon.html
 [browser_oversized.js]
 support-files =
   large_favicon.html
   large.png
+[browser_favicon_accept.js]
+support-files =
+  accept.html
+  accept.sjs
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/favicons/browser_favicon_accept.js
@@ -0,0 +1,20 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const ROOT = getRootDirectory(gTestPath).replace("chrome://mochitest/content/", "http://mochi.test:8888/");
+
+add_task(async () => {
+  await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" }, async (browser) => {
+    let faviconPromise = waitForFaviconMessage(true, `${ROOT}accept.sjs`);
+
+    BrowserTestUtils.loadURI(browser, ROOT + "accept.html");
+    await BrowserTestUtils.browserLoaded(browser);
+
+    try {
+      let result = await faviconPromise;
+      Assert.equal(result.iconURL, `${ROOT}accept.sjs`, "Should have seen the icon");
+    } catch (e) {
+      Assert.ok(false, "Favicon load failed.");
+    }
+  });
+});
--- a/browser/modules/FaviconLoader.jsm
+++ b/browser/modules/FaviconLoader.jsm
@@ -103,16 +103,25 @@ class FaviconLoad {
     this.channel.loadFlags |= Ci.nsIRequest.LOAD_BACKGROUND |
                               Ci.nsIRequest.VALIDATE_NEVER |
                               Ci.nsIRequest.LOAD_FROM_CACHE;
     // Sometimes node is a document and sometimes it is an element. This is
     // the easiest single way to get to the load group in both those cases.
     this.channel.loadGroup = iconInfo.node.ownerGlobal.document.documentLoadGroup;
     this.channel.notificationCallbacks = this;
 
+    if (this.channel instanceof Ci.nsIHttpChannel) {
+      try {
+        let acceptHeader = Services.prefs.getCharPref("image.http.accept");
+        this.channel.setRequestHeader("Accept", acceptHeader, false);
+      } catch (e) {
+        // Failing to get the pref or set the header is ignorable.
+      }
+    }
+
     if (Services.prefs.getBoolPref("network.http.tailing.enabled", true) &&
         this.channel instanceof Ci.nsIClassOfService) {
       this.channel.addClassFlags(Ci.nsIClassOfService.Tail | Ci.nsIClassOfService.Throttleable);
     }
   }
 
   load() {
     this._deferred = PromiseUtils.defer();