From 822b8bf71daff6f309aa2094c192412982cc07f6 Mon Sep 17 00:00:00 2001
From: "mozilla.mano%sent.com" <mozilla.mano%sent.com>
Date: Wed, 9 Jan 2008 04:07:42 +0000
Subject: [PATCH] Bug 302575 - URL bar gets confused about what URI is loaded.
 patch by Florian Queze <florian@mozilla.com> and Gavin Sharp. r=gavin,
 sr=neil.

---
 browser/base/content/browser.js     | 28 +++--------
 browser/base/content/tabbrowser.xml | 11 +++--
 toolkit/content/widgets/browser.xml | 77 ++++++++++++++++++++++++++---
 3 files changed, 85 insertions(+), 31 deletions(-)

diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
index 0b36327c039..4ba854a4b62 100644
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1323,7 +1323,7 @@ function gotoHistoryIndex(aEvent)
     // Normal click.  Go there in the current tab and update session history.
 
     try {
-      getWebNavigation().gotoIndex(index);
+      getBrowser().gotoIndex(index);
     }
     catch(ex) {
       return false;
@@ -1348,7 +1348,7 @@ function BrowserForward(aEvent, aIgnoreAlt)
 
   if (where == "current") {
     try {
-      getWebNavigation().goForward();
+      getBrowser().goForward();
     }
     catch(ex) {
     }
@@ -1368,7 +1368,7 @@ function BrowserBack(aEvent, aIgnoreAlt)
 
   if (where == "current") {
     try {
-      getWebNavigation().goBack();
+      getBrowser().goBack();
     }
     catch(ex) {
     }
@@ -1611,7 +1611,7 @@ function loadURI(uri, referrer, postData, allowThirdPartyFixup)
     if (allowThirdPartyFixup) {
       flags = nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
     }
-    getWebNavigation().loadURI(uri, flags, referrer, postData, null);
+    getBrowser().loadURIWithFlags(uri, flags, referrer, null, postData);
   } catch (e) {
   }
 }
@@ -3475,10 +3475,6 @@ nsBrowserStatusHandler.prototype =
         this.reloadSkipCacheCommand.removeAttribute("disabled");
       }
 
-      // The document loaded correctly, clear the value if we should
-      if (browser.userTypedClear > 0 && aRequest)
-        browser.userTypedValue = null;
-
       if (!gBrowser.mTabbedMode && aWebProgress.isLoadingDocument)
         gBrowser.setIcon(gBrowser.mCurrentTab, null);
 
@@ -3719,13 +3715,6 @@ nsBrowserStatusHandler.prototype =
 
   startDocumentLoad : function(aRequest)
   {
-    // It's okay to clear what the user typed when we start
-    // loading a document. If the user types, this counter gets
-    // set to zero, if the document load ends without an
-    // onLocationChange, this counter gets decremented
-    // (so we keep it while switching tabs after failed loads)
-    getBrowser().userTypedClear++;
-
     // clear out feed data
     gBrowser.mCurrentBrowser.feeds = null;
 
@@ -3744,11 +3733,6 @@ nsBrowserStatusHandler.prototype =
 
   endDocumentLoad : function(aRequest, aStatus)
   {
-    // The document is done loading, we no longer want the
-    // value cleared.
-    if (getBrowser().userTypedClear > 0)
-      getBrowser().userTypedClear--;
-
     const nsIChannel = Components.interfaces.nsIChannel;
     var urlStr = aRequest.QueryInterface(nsIChannel).originalURI.spec;
 
@@ -3856,8 +3840,8 @@ nsBrowserAccess.prototype =
                                 .QueryInterface(Ci.nsIInterfaceRequestor)
                                 .getInterface(Ci.nsIDOMWindow);
             if (aURI) {
-              getWebNavigation().loadURI(aURI.spec, loadflags, null, 
-                                         null, null);
+              gBrowser.loadURIWithFlags(aURI.spec, loadflags, null, 
+                                        null, null);
             }
           }
           if(!gPrefService.getBoolPref("browser.tabs.loadDivertedInBackground"))
diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
index ad9450170b1..c843ef5a953 100644
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -344,8 +344,11 @@
                 // set to zero, if the document load ends without an
                 // onLocationChange, this counter gets decremented
                 // (so we keep it while switching tabs after failed loads)
+                // We need to add 2 because loadURIWithFlags may have
+                // cancelled a pending load which would have cleared
+                // its anchor scroll detection temporary increment.
                 if (aWebProgress.DOMWindow == this.mBrowser.contentWindow)
-                  this.mBrowser.userTypedClear++;
+                  this.mBrowser.userTypedClear += 2;
 
                 if (!this.mBlank) {
                   this.mTab.setAttribute("busy", "true");
@@ -361,7 +364,9 @@
                 if (aWebProgress.DOMWindow == this.mBrowser.contentWindow) {
                   // The document is done loading, we no longer want the
                   // value cleared.
-                  if (this.mBrowser.userTypedClear > 0)
+                  if (this.mBrowser.userTypedClear > 1)
+                    this.mBrowser.userTypedClear -= 2;
+                  else if (this.mBrowser.userTypedClear > 0)
                     this.mBrowser.userTypedClear--;
 
                   if (!this.mBrowser.mIconURL)
@@ -411,7 +416,7 @@
             onLocationChange : function(aWebProgress, aRequest, aLocation)
             {
               // The document loaded correctly, clear the value if we should
-              if (this.mBrowser.userTypedClear > 0 && aRequest)
+              if (this.mBrowser.userTypedClear > 0)
                 this.mBrowser.userTypedValue = null;
 
               if (aWebProgress.DOMWindow == this.mBrowser.contentWindow &&
diff --git a/toolkit/content/widgets/browser.xml b/toolkit/content/widgets/browser.xml
index 03819e80510..08ee618fdd6 100644
--- a/toolkit/content/widgets/browser.xml
+++ b/toolkit/content/widgets/browser.xml
@@ -89,8 +89,15 @@
         <body>
           <![CDATA[
             var webNavigation = this.webNavigation;
-            if (webNavigation.canGoBack)
-              webNavigation.goBack();
+            if (webNavigation.canGoBack) {
+              try {
+                this.userTypedClear++;
+                webNavigation.goBack();
+              } finally {
+                if (this.userTypedClear)
+                  this.userTypedClear--;
+              }
+            }
           ]]>
         </body>
       </method>
@@ -99,8 +106,15 @@
         <body>
           <![CDATA[
             var webNavigation = this.webNavigation;
-            if (webNavigation.canGoForward)
-              webNavigation.goForward();
+            if (webNavigation.canGoForward) {
+              try {
+                this.userTypedClear++;
+                webNavigation.goForward();
+              } finally {
+                if (this.userTypedClear)
+                  this.userTypedClear--;
+              }
+            }
           ]]>
         </body>
       </method>
@@ -168,7 +182,13 @@
               }
             }
             this.mIconURL = null;
-            this.webNavigation.loadURI(aURI, aFlags, aReferrerURI, aPostData, null);
+            try {
+              this.userTypedClear++;
+              this.webNavigation.loadURI(aURI, aFlags, aReferrerURI, aPostData, null);
+            } finally {
+              if (this.userTypedClear)
+                this.userTypedClear--;
+            }
           ]]>
         </body>
       </method>
@@ -210,7 +230,13 @@
         <parameter name="aIndex"/>
         <body>
           <![CDATA[
-            this.webNavigation.gotoIndex(aIndex);
+            try {
+              this.userTypedClear++;
+              this.webNavigation.gotoIndex(aIndex);
+            } finally {
+              if (this.userTypedClear)
+                this.userTypedClear--;
+            }
           ]]>
         </body>
       </method>
@@ -535,6 +561,45 @@
         </setter>
       </property>
 
+      <!--
+        This field tracks the location bar state. The value that the user typed
+        in to the location bar may not be changed while this field is zero.
+        However invoking a load will temporarily increase this field to allow
+        the location bar to be updated to the new URL.
+
+        Case 1: Anchor scroll
+          The user appends the anchor to the URL. This sets the location bar
+          into typed state, and disables changes to the location bar. The user
+          then requests the scroll. loadURIWithFlags temporarily increases the
+          flag by 1 so that the anchor scroll's location change resets the
+          location bar state.
+
+        Case 2: Interrupted load
+          The user types in and submits the URL. This triggers an asynchronous
+          network load which increases the flag by 2. (The temporary increase
+          from loadURIWithFlags is not noticeable in this case.) When the load
+          is interrupted the flag returns to zero, and the location bar stays
+          in typed state.
+
+        Case 3: New load
+          This works like case 2, but as the load is not interrupted the
+          location changes while the flag is still 2 thus resetting the
+          location bar state.
+
+        Case 4: Corrected load
+          This is a combination of case 2 and case 3, except that the original
+          load is interrupted by the new load. Normally cancelling and starting
+          a new load would reset the flag to 0 and then increase it to 2 again.
+          However both actions occur as a consequence of the loadURIWithFlags
+          invocation, which adds its temporary increase in to the mix. Since
+          the new URL would have been typed in the flag would have been reset
+          before loadURIWithFlags incremented it. The interruption resets the
+          flag to 0 and increases it to 2. Although loadURIWithFlags will
+          decrement the flag it remains at 1 thus allowing the location bar
+          state to be reset when the new load changes the location.
+          This case also applies when loading into a new browser, as this
+          interrupts the default load of about:blank.
+      -->
       <field name="userTypedClear">
         1
       </field>