Bug 411966 – Wrong favicon appears in the bookmarks list. r=dietrich
☠☠ backed out by a53ccaba796e ☠ ☠
authorMarco Bonardo <mak77@bonardo.net>
Wed, 13 Aug 2008 08:44:38 +0200
changeset 16612 be5521120b0618f5167363a16f074114e0c4ff75
parent 16611 9c07908185e14ec69c192b872d2c3ae99986ee4c
child 16613 603f2fcb6a54e4f2dfbde726c7b42c1fede2c20c
child 16616 a53ccaba796ebdb181a9f0e193e8fb818dd03034
push idunknown
push userunknown
push dateunknown
reviewersdietrich
bugs411966
milestone1.9.1a2pre
Bug 411966 – Wrong favicon appears in the bookmarks list. r=dietrich
toolkit/components/places/src/nsNavHistory.cpp
toolkit/components/places/tests/Makefile.in
toolkit/components/places/tests/mochitest/test_bug_411966.html
toolkit/components/places/tests/mochitest/test_bug_411966/ClickedPage.htm
toolkit/components/places/tests/mochitest/test_bug_411966/ClickedPage.htm^headers^
toolkit/components/places/tests/mochitest/test_bug_411966/PermRedirectPage.htm
toolkit/components/places/tests/mochitest/test_bug_411966/TempRedirectPage.htm
toolkit/components/places/tests/mochitest/test_bug_411966/TempRedirectPage.htm^headers^
toolkit/components/places/tests/mochitest/test_bug_411966/TypedPage.htm
toolkit/components/places/tests/mochitest/test_bug_411966/makefile.in
toolkit/components/places/tests/mochitest/test_bug_411966/redirect.js
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -18,17 +18,17 @@
  * Portions created by the Initial Developer are Copyright (C) 2005
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *   Brett Wilson <brettw@gmail.com> (original author)
  *   Dietrich Ayala <dietrich@mozilla.com>
  *   Seth Spitzer <sspitzer@mozilla.com>
  *   Asaf Romano <mano@mozilla.com>
- *   Marco Bonardo <mak77@supereva.it>
+ *   Marco Bonardo <mak77@bonardo.net>
  *   Edward Lee <edward.lee@engineering.uiuc.edu>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
@@ -4200,22 +4200,23 @@ nsNavHistory::AddURIInternal(nsIURI* aUR
 //    there are any redirects that are bookmarks the specs will be placed in
 //    this buffer. The caller can then determine if any bookmarked items were
 //    visited so it knows whether to update the bookmark service's redirect
 //    hashtable.
 
 nsresult
 nsNavHistory::AddVisitChain(nsIURI* aURI, PRTime aTime,
                             PRBool aToplevel, PRBool aIsRedirect,
-                            nsIURI* aReferrer, PRInt64* aVisitID,
+                            nsIURI* aReferrerURI, PRInt64* aVisitID,
                             PRInt64* aSessionID, PRInt64* aRedirectBookmark)
 {
   PRUint32 transitionType = 0;
   PRInt64 referringVisit = 0;
   PRTime visitTime = 0;
+  nsCOMPtr<nsIURI> fromVisitURI = aReferrerURI;
 
   nsCAutoString spec;
   nsresult rv = aURI->GetSpec(spec);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCAutoString redirectSource;
   if (GetRedirectFor(spec, redirectSource, &visitTime, &transitionType)) {
     // this was a redirect: See GetRedirectFor for info on how this works
@@ -4232,31 +4233,35 @@ nsNavHistory::AddVisitChain(nsIURI* aURI
       GetUrlIdFor(redirectURI, aRedirectBookmark, PR_FALSE);
     }
 
     // Find the visit for the source. Note that we decrease the time counter,
     // which will ensure that the referrer and this page will appear in history
     // in the correct order. Since the times are in microseconds, it should not
     // normally be possible to get two pages within one microsecond of each
     // other so the referrer won't appear before a previous page viewed.
-    rv = AddVisitChain(redirectURI, aTime - 1, aToplevel, PR_TRUE, aReferrer,
+    rv = AddVisitChain(redirectURI, aTime - 1, aToplevel, PR_TRUE, aReferrerURI,
                        &referringVisit, aSessionID, aRedirectBookmark);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // for redirects in frames, we don't want to see those items in history
     // see bug #381453 for more details
     if (!aToplevel) {
       transitionType = nsINavHistoryService::TRANSITION_EMBED;
     }
-  } else if (aReferrer) {
+
+    // We have been redirected, update the referrer so we can walk up
+    // the redirect chain. See bug 411966 for details.
+    fromVisitURI = redirectURI;
+  } else if (aReferrerURI) {
     // We do not want to add a new visit if the referring site is the same as
     // the new site.  This is the situation where a page refreshes itself to
     // give the user updated information.
     PRBool referrerIsSame;
-    if (NS_SUCCEEDED(aURI->Equals(aReferrer, &referrerIsSame)) && referrerIsSame)
+    if (NS_SUCCEEDED(aURI->Equals(aReferrerURI, &referrerIsSame)) && referrerIsSame)
       return NS_OK;
 
     // If there is a referrer, we know you came from somewhere, either manually
     // or automatically. For toplevel windows, assume its manual and you want
     // to see this in history. For other things, it's some kind of embedded
     // navigation. This is true of images and other content the user doesn't
     // want to see in their history, but also of embedded frames that the user
     // navigated manually and probably DOES want to see in history.
@@ -4276,17 +4281,17 @@ nsNavHistory::AddVisitChain(nsIURI* aURI
     // caches the value of "now" until next time the event loop runs. This
     // gives better performance, but here we may get many notifications without
     // running the event loop. We must preserve these events' ordering. This
     // most commonly happens on redirects.
     visitTime = PR_Now();
 
     // Try to turn the referrer into a visit.
     // This also populates the session id.
-    if (!FindLastVisit(aReferrer, &referringVisit, aSessionID)) {
+    if (!FindLastVisit(aReferrerURI, &referringVisit, aSessionID)) {
       // we couldn't find a visit for the referrer, don't set it
       *aSessionID = GetNewSessionID();
     }
   } else {
     // When there is no referrer, we know the user must have gotten the link
     // from somewhere, so check our sources to see if it was recently typed or
     // has a bookmark selected. We don't handle drag-and-drop operations.
     // note:  the link may have also come from a new window (set to load a homepage)
@@ -4303,17 +4308,17 @@ nsNavHistory::AddVisitChain(nsIURI* aURI
     else
       transitionType = nsINavHistoryService::TRANSITION_EMBED;
 
     visitTime = PR_Now();
     *aSessionID = GetNewSessionID();
   }
 
   // this call will create the visit and create/update the page entry
-  return AddVisit(aURI, visitTime, aReferrer, transitionType,
+  return AddVisit(aURI, visitTime, fromVisitURI, transitionType,
                   aIsRedirect, *aSessionID, aVisitID);
 }
 
 
 // nsNavHistory::IsVisited
 //
 //    Note that this ignores the "hidden" flag. This function just checks if the
 //    given page is in the DB for link coloring. The "hidden" flag affects
--- a/toolkit/components/places/tests/Makefile.in
+++ b/toolkit/components/places/tests/Makefile.in
@@ -50,24 +50,26 @@ XPCSHELL_TESTS = \
                  autocomplete \
                  bookmarks \
                  queries \
                  unit \
                  $(NULL)
 
 # Simple MochiTests
 MOCHI_TESTS = mochitest/test_bug_405924.html \
+              mochitest/test_bug_411966.html \
               $(NULL)
 
 MOCHI_CONTENT = mochitest/prompt_common.js \
                 $(NULL)
 
 ifdef MOZ_MOCHITEST
 DIRS = \
   chrome \
+  mochitest/test_bug_411966 \
 # These tests are disabled for the time being, see bug 416066
 #  browser \
   $(NULL)
 endif
 
 include $(topsrcdir)/config/rules.mk
 
 libs:: $(MOCHI_TESTS) $(MOCHI_CONTENT)
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966.html
@@ -0,0 +1,40 @@
+<!DOCTYPE HTML>
+<html>
+  <!--
+    https://bugzilla.mozilla.org/show_bug.cgi?id=411966
+  -->
+  <head>
+    <title>Test for Bug 411966</title>
+    <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+    <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+    <script type="text/javascript" src="test_bug_411966/redirect.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  </head>
+  <body>
+    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=411966">
+      Mozilla Bug 411966</a>
+    <p id="display"></p>
+    <div id="content" style="display: none">
+      <iframe id="iframe"></iframe>
+    </div>
+    <pre id="test">
+      <script class="testbody" type="text/javascript">
+
+      /** Test for Bug 411966 **/
+      netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+      ghist.addURI(typedURI, false, true, null);
+      bhist.markPageAsTyped(typedURI);
+
+      var clickedLinkChannel = ios.newChannelFromURI(clickedLinkURI);
+      clickedLinkChannel.QueryInterface(Ci.nsIHttpChannel).referrer = typedURI;
+      var listener = new StreamListener(clickedLinkChannel, checkDB);
+      clickedLinkChannel.notificationCallbacks = listener;
+      clickedLinkChannel.asyncOpen(listener, null);
+
+      SimpleTest.waitForExplicitFinish();
+
+     </script>
+   </pre>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/ClickedPage.htm
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>Bug 411966</title>
+  </head>
+  </body>
+    Clicked link page!
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/ClickedPage.htm^headers^
@@ -0,0 +1,2 @@
+HTTP 302 Moved Temporarily
+Location: http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/TempRedirectPage.htm
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/PermRedirectPage.htm
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>Bug 411966</title>
+  </head>
+  </body>
+    Permanently redirected!
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/TempRedirectPage.htm
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>Bug 411966</title>
+  </head>
+  </body>
+    Temporarly redirected!
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/TempRedirectPage.htm^headers^
@@ -0,0 +1,2 @@
+HTTP 301 Moved Permanently
+Location: http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/PermRedirectPage.htm
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/TypedPage.htm
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>Bug 411966</title>
+  </head>
+  </body>
+    Typed in page!
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/makefile.in
@@ -0,0 +1,59 @@
+#
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is Bug 411966 test code.
+#
+# The Initial Developer of the Original Code is
+# Mozilla.org.
+# Portions created by the Initial Developer are Copyright (C) 2005
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#   Marco Bonardo <mak77@bonardo.net> (Original Author)
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH = ../../../../../..
+topsrcdir = @top_srcdir@
+srcdir = @srcdir@
+VPATH = @srcdir@
+relativesrcdir = toolkit/components/places/tests/test_bug_411966
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_HTTP_FILES = \
+    redirect.js \
+    TypedPage.htm \
+    ClickedPage.htm \
+    ClickedPage.htm^headers^ \
+    TempRedirectPage.htm \
+    TempRedirectPage.htm^headers^ \
+    PermRedirectPage.htm \
+    $(NULL)
+
+libs:: $(_HTTP_FILES)
+  $(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/mochitest/test_bug_411966/redirect.js
@@ -0,0 +1,191 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Bug 411966 test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2008
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Marco Bonardo <mak77@bonardo.net> (Original Author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+const Ci = Components.interfaces;
+ok(Ci != null, "Access Ci");
+const Cc = Components.classes;
+ok(Cc != null, "Access Cc");
+
+// Get Services.
+var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+              getService(Ci.nsINavHistoryService);
+ok(histsvc != null, "Could not get History Service");
+var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
+ok(bhist != null, "Could not get Browser History Service");
+var ghist = Cc["@mozilla.org/browser/global-history;2"].
+            getService(Ci.nsIGlobalHistory2);
+ok(ghist != null, "Could not get Global History Service");
+var ghist3 = ghist.QueryInterface(Ci.nsIGlobalHistory3);
+ok(ghist3 != null, "Could not get Global History Service");
+var ios = Cc["@mozilla.org/network/io-service;1"].
+          getService(Components.interfaces.nsIIOService);
+ok(ios != null, "Could not get IO Service");
+var dirSvc = Cc["@mozilla.org/file/directory_service;1"].
+             getService(Ci.nsIProperties);
+ok(dirSvc != null, "Could not get Directory Service");
+var storage = Cc["@mozilla.org/storage/service;1"].
+              getService(Ci.mozIStorageService);
+ok(storage != null, "Could not get Storage Service");
+
+// Get database connection.
+var database = dirSvc.get('ProfD', Ci.nsIFile);
+database.append("places.sqlite");
+var mDBConn = storage.openDatabase(database);
+ok(mDBConn != null, "Could not get Database Connection");
+
+function uri(URIString) {
+  return ios.newURI(URIString, null, null);
+}
+
+var typedURI = uri("http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/TypedPage.htm");
+var clickedLinkURI = uri("http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/ClickedPage.htm");
+var temporaryRedirectURI = uri("http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/TempRedirectPage.htm");
+var permanentRedirectURI = uri("http://localhost:8888/tests/toolkit/components/places/tests/test_bug_411966/PermRedirectPage.htm");
+
+// Stream Listener
+function StreamListener(aChannel, aCallbackFunc) {
+  this.mChannel = aChannel;
+  this.mCallbackFunc = aCallbackFunc;
+}
+
+StreamListener.prototype = {
+  mData: "",
+  mChannel: null,
+
+  // nsIStreamListener
+  onStartRequest: function (aRequest, aContext) {
+    this.mData = "";
+  },
+
+  onDataAvailable: function (aRequest, aContext, aStream, aSourceOffset, aLength) {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    // We actually don't need received data
+    var scriptableInputStream =
+      Components.classes["@mozilla.org/scriptableinputstream;1"]
+                .createInstance(Components.interfaces.nsIScriptableInputStream);
+    scriptableInputStream.init(aStream);
+
+    this.mData += scriptableInputStream.read(aLength);
+  },
+
+  onStopRequest: function (aRequest, aContext, aStatus) {
+    if (Components.isSuccessCode(aStatus))
+      this.mCallbackFunc(this.mData);
+    else
+      throw("Could not get page.");
+
+    this.mChannel = null;
+  },
+
+  // nsIChannelEventSink
+  onChannelRedirect: function (aOldChannel, aNewChannel, aFlags) {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    ghist3.addDocumentRedirect(aOldChannel, aNewChannel, aFlags, true);
+    // If redirecting, store the new channel
+    this.mChannel = aNewChannel;
+  },
+
+  // nsIInterfaceRequestor
+  getInterface: function (aIID) {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    try {
+      return this.QueryInterface(aIID);
+    } catch (e) {
+      throw Components.results.NS_NOINTERFACE;
+    }
+  },
+
+  // nsIProgressEventSink (not implementing will cause annoying exceptions)
+  onProgress : function (aRequest, aContext, aProgress, aProgressMax) { },
+  onStatus : function (aRequest, aContext, aStatus, aStatusArg) { },
+
+  // nsIHttpEventSink (not implementing will cause annoying exceptions)
+  onRedirect : function (aOldChannel, aNewChannel) { },
+
+  // we are faking an XPCOM interface, so we need to implement QI
+  QueryInterface : function(aIID) {
+    if (aIID.equals(Components.interfaces.nsISupports) ||
+        aIID.equals(Components.interfaces.nsIInterfaceRequestor) ||
+        aIID.equals(Components.interfaces.nsIChannelEventSink) ||
+        aIID.equals(Components.interfaces.nsIProgressEventSink) ||
+        aIID.equals(Components.interfaces.nsIHttpEventSink) ||
+        aIID.equals(Components.interfaces.nsIStreamListener))
+      return this;
+
+    throw Components.results.NS_NOINTERFACE;
+  }
+};
+
+// Check Callback.
+function checkDB(data){
+  netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+  var referrer = this.mChannel.QueryInterface(Ci.nsIHttpChannel).referrer;
+  ghist.addURI(this.mChannel.URI, true, true, referrer);
+
+  // We have to wait since we use lazy_add, lazy_timer is 3s
+  setTimeout("checkDBOnTimeout()", 4000);
+}
+
+function checkDBOnTimeout() {
+  netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+  // Get all pages visited from the original typed one
+  var sql = "SELECT url FROM moz_historyvisits " +
+            "JOIN moz_places h ON h.id = place_id " +
+            "WHERE from_visit IN " +
+              "(SELECT v.id FROM moz_historyvisits v " +
+              "JOIN moz_places p ON p.id = v.place_id " +
+              "WHERE p.url = ?1)";
+  var stmt = mDBConn.createStatement(sql);
+  stmt.bindUTF8StringParameter(0, typedURI.spec);
+
+  var empty = true;
+  while (stmt.executeStep()) {
+    empty = false;
+    var visitedURI = stmt.getUTF8String(0);
+    // Check that redirect from_visit is not from the original typed one
+    ok(visitedURI == clickedLinkURI.spec, "Got wrong referrer for " + visitedURI);
+  }
+  // Ensure that we got some result
+  ok(!empty, "empty table");
+
+  SimpleTest.finish();
+}