Bug 577892 - allow icons to change when href attribute is set directly, with automated test, r=dolske,bz
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 03 Jul 2014 22:40:52 +0100
changeset 193488 1829aad143e242ef8f395060e966c6130383978a
parent 193487 0729de0a4a0214a8a0e9142e9fb4ef590663c29a
child 193489 4ce737d7339d21ff48979578d90cfa9688302623
push id27122
push userryanvm@gmail.com
push dateFri, 11 Jul 2014 20:22:43 +0000
treeherderautoland@1ddc3c59e82b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske, bz
bugs577892
milestone33.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 577892 - allow icons to change when href attribute is set directly, with automated test, r=dolske,bz
browser/base/content/browser.js
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_favicon_change.js
browser/base/content/test/general/file_favicon_change.html
browser/modules/ContentLinkHandler.jsm
content/html/content/src/HTMLLinkElement.cpp
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2837,38 +2837,38 @@ var newWindowButtonObserver = {
     });
   }
 }
 
 const DOMLinkHandler = {
   init: function() {
     let mm = window.messageManager;
     mm.addMessageListener("Link:AddFeed", this);
-    mm.addMessageListener("Link:AddIcon", this);
+    mm.addMessageListener("Link:SetIcon", this);
     mm.addMessageListener("Link:AddSearch", this);
   },
 
   receiveMessage: function (aMsg) {
     switch (aMsg.name) {
       case "Link:AddFeed":
         let link = {type: aMsg.data.type, href: aMsg.data.href, title: aMsg.data.title};
         FeedHandler.addFeed(link, aMsg.target);
         break;
 
-      case "Link:AddIcon":
-        return this.addIcon(aMsg.target, aMsg.data.url);
+      case "Link:SetIcon":
+        return this.setIcon(aMsg.target, aMsg.data.url);
         break;
 
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
     }
   },
 
-  addIcon: function(aBrowser, aURL) {
+  setIcon: function(aBrowser, aURL) {
     if (gBrowser.isFailedIcon(aURL))
       return false;
 
     let tab = gBrowser._getTabForBrowser(aBrowser);
     if (!tab)
       return false;
 
     gBrowser.setIcon(tab, aURL);
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -48,16 +48,17 @@ support-files =
   file_bug906190_redirected.html
   file_bug906190.js
   file_bug906190.sjs
   file_bug970276_popup1.html
   file_bug970276_popup2.html
   file_bug970276_favicon1.ico
   file_bug970276_favicon2.ico
   file_dom_notifications.html
+  file_favicon_change.html
   file_fullscreen-window-open.html
   get_user_media.html
   head.js
   healthreport_testRemoteCommands.html
   moz.png
   offlineQuotaNotification.cacheManifest
   offlineQuotaNotification.html
   page_style_sample.html
@@ -285,16 +286,17 @@ run-if = datareporting
 skip-if = (os == "linux" && debug) || e10s # linux: bug 976544; e10s: Bug 973001 - appears user media notifications only happen in the child and don't make their way to the parent?
 [browser_devices_get_user_media_about_urls.js]
 skip-if = e10s # Bug 973001 - appears user media notifications only happen in the child and don't make their way to the parent?
 [browser_discovery.js]
 skip-if = e10s # Bug 918663 - DOMLinkAdded events don't make their way to chrome
 [browser_duplicateIDs.js]
 [browser_drag.js]
 skip-if = true # browser_drag.js is disabled, as it needs to be updated for the new behavior from bug 320638.
+[browser_favicon_change.js]
 [browser_findbarClose.js]
 skip-if = e10s # Bug ?????? - test directly manipulates content (tries to grab an iframe directly from content)
 [browser_fullscreen-window-open.js]
 skip-if = e10s || os == "linux" # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly. Linux: Intermittent failures - bug 941575.
 [browser_gestureSupport.js]
 skip-if = e10s # Bug 863514 - no gesture support.
 [browser_getshortcutoruri.js]
 [browser_hide_removing.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_favicon_change.js
@@ -0,0 +1,40 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const TEST_URL = "http://mochi.test:8888/browser/browser/base/content/test/general/file_favicon_change.html"
+
+add_task(function*() {
+  let extraTab = gBrowser.selectedTab = gBrowser.addTab();
+  let tabLoaded = promiseTabLoaded(extraTab);
+  extraTab.linkedBrowser.loadURI(TEST_URL);
+  let expectedFavicon = "http://example.org/one-icon";
+  let haveChanged = new Promise.defer();
+  let observer = new MutationObserver(function(mutations) {
+    for (let mut of mutations) {
+      if (mut.attributeName != "image") {
+        continue;
+      }
+      let imageVal = extraTab.getAttribute("image").replace(/#.*$/, "");
+      if (!imageVal) {
+        // The value gets removed because it doesn't load.
+        continue;
+      }
+      is(imageVal, expectedFavicon, "Favicon image should correspond to expected image.");
+      haveChanged.resolve();
+    }
+  });
+  observer.observe(extraTab, {attributes: true});
+  yield tabLoaded;
+  yield haveChanged.promise;
+  haveChanged = new Promise.defer();
+  expectedFavicon = "http://example.org/other-icon";
+  let contentWin = extraTab.linkedBrowser.contentWindow;
+  let ev = new contentWin.CustomEvent("PleaseChangeFavicon", {});
+  contentWin.dispatchEvent(ev);
+  yield haveChanged.promise;
+  observer.disconnect();
+  gBrowser.removeTab(extraTab);
+});
+
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/file_favicon_change.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html><head>
+  <meta http-equiv="content-type" content="text/html; charset=utf-8">
+  <link rel="icon" href="http://example.org/one-icon" type="image/ico" id="i">
+</head>
+<body>
+  <script>
+  window.addEventListener("PleaseChangeFavicon", function() {
+    var ico = document.getElementById("i");
+    ico.setAttribute("href", "http://example.org/other-icon");
+  });
+  </script>
+</body></html>
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -16,21 +16,24 @@ Cu.import("resource://gre/modules/Servic
 XPCOMUtils.defineLazyModuleGetter(this, "Feeds",
   "resource:///modules/Feeds.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
   "resource://gre/modules/BrowserUtils.jsm");
 
 this.ContentLinkHandler = {
   init: function(chromeGlobal) {
     chromeGlobal.addEventListener("DOMLinkAdded", (event) => {
-      this.onLinkAdded(event, chromeGlobal);
+      this.onLinkEvent(event, chromeGlobal);
+    }, false);
+    chromeGlobal.addEventListener("DOMLinkChanged", (event) => {
+      this.onLinkEvent(event, chromeGlobal);
     }, false);
   },
 
-  onLinkAdded: function(event, chromeGlobal) {
+  onLinkEvent: function(event, chromeGlobal) {
     var link = event.originalTarget;
     var rel = link.rel && link.rel.toLowerCase();
     if (!link || !link.ownerDocument || !rel || !link.href)
       return;
 
     // Ignore sub-frames (bugs 305472, 479408).
     let window = link.ownerDocument.defaultView;
     if (window != window.top)
@@ -42,17 +45,17 @@ this.ContentLinkHandler = {
     var rels = {};
     for (let relString of rel.split(/\s+/))
       rels[relString] = true;
 
     for (let relVal in rels) {
       switch (relVal) {
         case "feed":
         case "alternate":
-          if (!feedAdded) {
+          if (!feedAdded && event.type == "DOMLinkAdded") {
             if (!rels.feed && rels.alternate && rels.stylesheet)
               break;
 
             if (Feeds.isValidFeed(link, link.ownerDocument.nodePrincipal, "feed" in rels)) {
               chromeGlobal.sendAsyncMessage("Link:AddFeed",
                                             {type: link.type,
                                              href: link.href,
                                              title: link.title});
@@ -64,21 +67,21 @@ this.ContentLinkHandler = {
           if (!iconAdded) {
             if (!Services.prefs.getBoolPref("browser.chrome.site_icons"))
               break;
 
             var uri = this.getLinkIconURI(link);
             if (!uri)
               break;
 
-            [iconAdded] = chromeGlobal.sendSyncMessage("Link:AddIcon", {url: uri.spec});
+            [iconAdded] = chromeGlobal.sendSyncMessage("Link:SetIcon", {url: uri.spec});
           }
           break;
         case "search":
-          if (!searchAdded) {
+          if (!searchAdded && event.type == "DOMLinkAdded") {
             var type = link.type && link.type.toLowerCase();
             type = type.replace(/^\s+|\s*(?:;.*)?$/g, "");
 
             let re = /^(?:https?|ftp):/i;
             if (type == "application/opensearchdescription+xml" && link.title &&
                 re.test(link.href))
             {
               let engine = { title: link.title, href: link.href };
--- a/content/html/content/src/HTMLLinkElement.cpp
+++ b/content/html/content/src/HTMLLinkElement.cpp
@@ -315,16 +315,17 @@ HTMLLinkElement::SetAttr(int32_t aNameSp
 
   // The ordering of the parent class's SetAttr call and Link::ResetLinkState
   // is important here!  The attribute is not set until SetAttr returns, and
   // we will need the updated attribute value because notifying the document
   // that content states have changed will call IntrinsicState, which will try
   // to get updated information about the visitedness from Link.
   if (aName == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
     Link::ResetLinkState(!!aNotify, true);
+    CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
   }
 
   if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None &&
       (aName == nsGkAtoms::href ||
        aName == nsGkAtoms::rel ||
        aName == nsGkAtoms::title ||
        aName == nsGkAtoms::media ||
        aName == nsGkAtoms::type)) {
@@ -377,16 +378,17 @@ HTMLLinkElement::UnsetAttr(int32_t aName
 
   // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState
   // is important here!  The attribute is not unset until UnsetAttr returns, and
   // we will need the updated attribute value because notifying the document
   // that content states have changed will call IntrinsicState, which will try
   // to get updated information about the visitedness from Link.
   if (aAttribute == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
     Link::ResetLinkState(!!aNotify, false);
+    CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
   }
 
   return rv;
 }
 
 nsresult
 HTMLLinkElement::PreHandleEvent(EventChainPreVisitor& aVisitor)
 {