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 512710 43c43fa35d2a
parent 512709 f4fc0e5c9d4a
child 512711 cf3598ee865b
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [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();