Last Comment Bug 302575 - URL bar gets confused about what URI is loaded
: URL bar gets confused about what URI is loaded
Status: VERIFIED FIXED
: fixed-seamonkey1.0
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: P3 normal with 2 votes (vote)
: Firefox 3 beta3
Assigned To: Florian Quèze [:florian] [:flo]
:
Mentors:
: 285622 313318 371935 376006 (view as bug list)
Depends on: 412841
Blocks: 282179 306810 306832
  Show dependency treegraph
 
Reported: 2005-07-28 17:21 PDT by Boris Zbarsky [:bz]
Modified: 2008-05-27 12:23 PDT (History)
25 users (show)
mbeltzner: blocking‑firefox3+
mtschrep: blocking‑firefox2-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Suite patch (2.29 KB, patch)
2005-08-29 07:17 PDT, neil@parkwaycc.co.uk
bzbarsky: review+
darin.moz: superreview+
Details | Diff | Splinter Review
firefox patch (3.19 KB, patch)
2005-08-30 17:20 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
darin.moz: superreview-
Details | Diff | Splinter Review
working Firefox patch (9.75 KB, patch)
2006-08-29 13:14 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Splinter Review
updated Firefox patch (9.60 KB, patch)
2007-09-12 10:01 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Splinter Review
Testcase for address bar editing (hash) (988 bytes, text/html)
2007-12-22 12:23 PST, Tim McCormack
no flags Details
updated Firefox patch (v5) (16.20 KB, patch)
2007-12-27 12:10 PST, Florian Quèze [:florian] [:flo]
gavin.sharp: review+
neil: superreview+
Details | Diff | Splinter Review
patch v6 (for checkin) (14.44 KB, patch)
2008-01-08 15:29 PST, Florian Quèze [:florian] [:flo]
no flags Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2005-07-28 17:21:01 PDT
STEPS TO REPRODUCE:

1)  Load https://bugzilla.mozilla.org/show_bug.cgi?id=213946
2)  Click in the URL bar, add "#29" to the end of the URI, and hit enter
3)  Click on the link that says #30

EXPECTED RESULTS:  URL bar says
https://bugzilla.mozilla.org/show_bug.cgi?id=213946#30

ACTUAL RESULTS:  URL bar says https://bugzilla.mozilla.org/show_bug.cgi?id=213946#29

I get the same result in Seamonkey and Firefox, but that's probably because of
code copy/paste.  I'm guessing the issue here is that our code that decides
whether the URL bar was edited looks for progress events that are not fired for
anchor scrolls or something silly like that...
Comment 1 James Ross 2005-07-28 17:27:46 PDT
I assume you mean "#c29" and "#c30" for the URL hash items?
Comment 2 Boris Zbarsky [:bz] 2005-07-28 17:48:09 PDT
er, yeah.  ;)
Comment 3 Darin Fisher 2005-07-28 22:35:38 PDT
Are we sure that nsIWebProgressListener::OnLocationChange is being fired?  If
so, then the UI must not be listening for that event.
Comment 4 Boris Zbarsky [:bz] 2005-07-28 23:41:25 PDT
The UI ignores OnLocationChange if it thinks the URI has been "edited by the
user".  My problem is with the way this last state is being determined.  Note
that if you never type anything in the URL bar, then it updates correctly when
scrolling to an anchor....
Comment 5 neil@parkwaycc.co.uk 2005-07-29 06:25:40 PDT
Do we also want the URL bar to update when a script executes an anchor scroll?
Comment 6 Boris Zbarsky [:bz] 2005-07-29 07:42:14 PDT
It does right now, no?
Comment 7 Boris Zbarsky [:bz] 2005-08-28 13:53:05 PDT
*** Bug 285622 has been marked as a duplicate of this bug. ***
Comment 8 Phil Ringnalda (:philor) 2005-08-28 23:50:44 PDT
So, when you type we set userTypedClear to 0, and increment it to say we can
change the displayed URI in startDocumentLoad, which only gets called for
STATE_START && STATE_IS_NETWORK - what else should we be listening for, to see,
um, state is scrolling to an anchor or maybe to the top of the document?

(Note to self: fixing this right ought to fix bug 227024 too)
Comment 9 Boris Zbarsky [:bz] 2005-08-29 06:00:54 PDT
I don't believe scrolling to an anchor fires anything other than onLocationChange.
Comment 10 neil@parkwaycc.co.uk 2005-08-29 07:17:24 PDT
Created attachment 194185 [details] [diff] [review]
Suite patch

My previous attempt at this patch dated back to the time when userTypedClear
was a boolean. Now that it's an integer the code is substantially simplified.
We temporarily bump userTypedClear in case an anchor load wants to reset the
location. Otherwise the document load starts, also bumping userTypedClear, so
that the URL bar will still get reset when the page eventually loads. If the
page is stopped before the location changes (e.g. errors with an alert) then
userTypedClear does not get reset thus avoiding dataloss.
Comment 11 Boris Zbarsky [:bz] 2005-08-30 13:44:03 PDT
Comment on attachment 194185 [details] [diff] [review]
Suite patch

So why does this help?
Comment 12 Boris Zbarsky [:bz] 2005-08-30 15:32:50 PDT
Comment on attachment 194185 [details] [diff] [review]
Suite patch

r=bzbarsky; I missed comment 10 somehow...
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-08-30 17:20:38 PDT
Created attachment 194390 [details] [diff] [review]
firefox patch
Comment 14 Darin Fisher 2005-08-31 13:35:24 PDT
Comment on attachment 194185 [details] [diff] [review]
Suite patch

sr=darin
Comment 15 Darin Fisher 2005-08-31 13:41:52 PDT
Comment on attachment 194390 [details] [diff] [review]
firefox patch

sr=darin
Comment 16 neil@parkwaycc.co.uk 2005-08-31 14:01:29 PDT
Fix checked in to the trunk.
Comment 17 Phil Ringnalda (:philor) 2005-08-31 14:45:44 PDT
Did anyone actually test this in Firefox? I tried porting it myself, and even
wrapping our other loadURI in browser.js, it still only sort-of worked, while
breaking other cases. Gavin's patch, which I assumed was a wip since he didn't
ask for any review, didn't do a single thing for me.
Comment 18 Darin Fisher 2005-08-31 15:44:57 PDT
Comment on attachment 194390 [details] [diff] [review]
firefox patch

oops.. ok.  i didn't try it :(
Comment 19 Phil Ringnalda (:philor) 2005-08-31 16:14:36 PDT
Verified busted in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
Gecko/20050831 Firefox/1.6a1 ID:2005083115 - for instance, follow bz's STR in a
new window with the tabbar hidden, you get no change in URL while clicking
anchors; then, ctrl-click a link to open a new background tab, and the urlbar is
marked dirty (default pageproxy icon, can't drag it).

Could someone please back out the Firefox patch?
Comment 20 Bruno 'Aqualon' Escherl 2005-09-02 10:13:54 PDT
The Suite patch caused bug 306810 as regression.

Bruno
Comment 21 Bruno 'Aqualon' Escherl 2005-09-02 23:33:39 PDT
I think the suite patch should be backed out too. With the patch favicons of
pages opened with "open in new tab" aren't visible and drag&drop of the URI to
the toolbar is broken (you can't get a grip at the icon before the URI).
Comment 22 Hermann Schwab 2005-09-04 05:28:55 PDT
(In reply to comment #21)
> I think the suite patch should be backed out too. With the patch favicons of
> pages opened with "open in new tab" aren't visible and drag&drop of the URI to
> the toolbar is broken (you can't get a grip at the icon before the URI).

That's Bug 306832 Bookmarks proxy icon (Favicon) not loaded when loading in a
new tab
Comment 23 Bruno 'Aqualon' Escherl 2005-09-04 08:36:10 PDT
The suite patch works with Neil's patch from Bug 306810. Bug 306832 gets fixed too.

Bruno
Comment 24 Phil Ringnalda (:philor) 2005-10-21 14:49:30 PDT
*** Bug 313318 has been marked as a duplicate of this bug. ***
Comment 25 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-12-14 08:29:40 PST
Comment on attachment 194185 [details] [diff] [review]
Suite patch

First a=me
Comment 26 Ian Neal 2005-12-14 09:18:28 PST
Comment on attachment 194185 [details] [diff] [review]
Suite patch

a=me for SM1.0b on SM only part of code inconjunction with patch for bug 306810, 2nd needed one
Comment 27 neil@parkwaycc.co.uk 2005-12-14 16:36:07 PST
SeaMonkey-only portion of patch checked in to the 1.8 branch.
Comment 28 Fredrik Branstrom 2006-08-18 04:49:59 PDT
So isn't this going to be fixed in 2.0?
Comment 29 Mike Connor [:mconnor] 2006-08-20 13:15:10 PDT
Gavin, can you clean up the patch here and get this fixed?
Comment 30 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-08-29 13:14:29 PDT
Created attachment 235957 [details] [diff] [review]
working Firefox patch

The tabbrowser implementations have diverged so much that the previous patch didn't have the intended effect. This one fixes this bug, by porting over more changes from the XPFE tabbrowser, but I haven't spent the time yet to verify that it won't have any unintended consequences, and if we really intend to make a change like this on the branch at this stage I'd like to be much more sure of it than I currently am. It does not regress bug 231393 in my testing.
Comment 31 Mike Schroepfer 2006-09-06 10:50:17 PDT
Given comment 30 and the time left in the release it doesn't look like we can finish this up for now so 181 drivers are taking this off the blocking list...
Comment 32 Phil Ringnalda (:philor) 2007-03-30 13:47:21 PDT
*** Bug 371935 has been marked as a duplicate of this bug. ***
Comment 33 Phil Ringnalda (:philor) 2007-03-30 13:47:30 PDT
*** Bug 376006 has been marked as a duplicate of this bug. ***
Comment 34 Robert Kaiser 2007-06-10 13:26:05 PDT
suite is now using toolkit's browser.xml with a port of the old xpfe tabbrowser to use that commone browser.xml - is this still fixed there?

And, don't we need to get the Firefox version in anyways some time?
Comment 35 Bruno 'Aqualon' Escherl 2007-06-10 13:37:11 PDT
It's broken again in Suiterunner (and probably still in Firefox).
Comment 36 Boris Zbarsky [:bz] 2007-06-10 13:55:04 PDT
Should probably be in toolkit so we can set the right blocking flags, fwiw...
Comment 37 neil@parkwaycc.co.uk 2007-06-11 06:04:30 PDT
(In reply to comment #34)
>suite is now using toolkit's browser.xml
Right, this is just one of the many changes that toolkit doesn't have, sigh...
Comment 38 Theo Ephraim 2007-09-07 09:57:52 PDT
This is still broken and a pain while trying to do some fancy AJAX back button hacking... Please someone take another crack at it.  Thanks!
Comment 39 neil@parkwaycc.co.uk 2007-09-10 04:18:50 PDT
Comment on attachment 235957 [details] [diff] [review]
working Firefox patch

I can confirm that the browser.xml changes refix the bug for SeaMonkey.
Comment 40 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-09-12 10:01:01 PDT
Created attachment 280617 [details] [diff] [review]
updated Firefox patch

I still don't fully understand these changes, but they seem to fix the bug.
Comment 41 neil@parkwaycc.co.uk 2007-09-12 10:12:17 PDT
(In reply to comment #40)
>I still don't fully understand these changes
browser.xml needs to tweak userTypedClear in case the URL is an anchor scroll; in this case there is no network activity associated with the location change so the normal userTypedClear does not occur.
tabbrowser.xml needs to tweak userTypedClear by 2 in case a document is loading; in this case the network stop/start resets userTypedClear to 2 so that browser.xml will only decrement it to 1 and the normal userTypedClear will occur.
Comment 42 Reed Loden [:reed] (use needinfo?) 2007-12-03 21:31:28 PST
What's the status of this bug? With Neil's comment, is Gavin's question answered. Ready for review?
Comment 43 Rostislav Hristov 2007-12-11 07:03:06 PST
Doesn't work here with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121105 Minefield/3.0b2pre

The issue is very annoying for everyone developing or using client-side deep linking.
Comment 44 Mike Connor [:mconnor] 2007-12-13 18:58:02 PST
-> florian
Comment 45 Florian Quèze [:florian] [:flo] 2007-12-21 17:02:31 PST
Gavin, do you know what was left to do here?

Apparently your last patch still applies. I'm going to test it to check that it still works. Maybe this comment should be added https://bugzilla.mozilla.org/show_bug.cgi?id=306810#c12
Comment 46 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-12-22 11:54:59 PST
From what I've been told my latest patch fixes the problem. The reason I haven't asked for review is because it was just a naive copy of the xpfe code, and I haven't had the time to verify that it's correct or even fully understand what it was changing. That comment from Neil in the other bug certainly looks useful, and he's really the expert on this code. I'm just concerned about any Firefox-specific effects due to differences in our tabbrowser.
Comment 47 Tim McCormack 2007-12-22 12:23:34 PST
Created attachment 294377 [details]
Testcase for address bar editing (hash)

Please check any patches against this testcase.
Comment 48 Dão Gottwald [:dao] 2007-12-23 03:22:18 PST
Comment on attachment 280617 [details] [diff] [review]
updated Firefox patch

>Index: browser/base/content/browser.js

>           // Setting the urlBar value in some cases causes userTypedValue to
>-          // become set because of oninput, so reset it to its old value.
>-          browser.userTypedValue = userTypedValue;
>+          // become set because of oninput, so reset it to null.
>+          browser.userTypedValue = null;

Please see bug 397815 where I'm removing this.
I don't think the hack in its current form is achieving anything. Changing it to delete userTypedValue seems like it could break stuff.

>Index: browser/base/content/tabbrowser.xml

>             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;

What is this change about? Without aRequest, there's no loading, so the comment isn't accurate anymore.
Comment 49 Florian Quèze [:florian] [:flo] 2007-12-27 12:10:18 PST
Created attachment 294676 [details] [diff] [review]
updated Firefox patch (v5)

Updated patch:
 * Added comments from the xpfe version of tabbrowser.xml

 * Replaced a few more getWebNavigation by getBrowser in browser.js

 * Removed the code touching the userTypedClear value from browser.js.  This seems to be duplicated code from tabbrowser.xml.  When I tested, the value of userTypedClear was always twice the value I expected because of this duplication.  I couldn't find any specific case that would execute this code from browser.js and not the same from tabbrowser.xml, please correct me if I'm wrong here :-).

Neil, do you know if there are cases where this code set the userTypedClear field to a non-zero value?
-                  if (this.mBrowser.userTypedClear > 0)
+                  if (this.mBrowser.userTypedClear > 1)
+                    this.mBrowser.userTypedClear -= 2;
+                  else if (this.mBrowser.userTypedClear > 0)
                     this.mBrowser.userTypedClear--;
If there are not, I would prefer to just set the value to 0, it would be more readable.
Comment 50 Florian Quèze [:florian] [:flo] 2007-12-27 12:17:17 PST
(In reply to comment #48)
> (From update of attachment 280617 [details] [diff] [review])
> >Index: browser/base/content/browser.js
> 
> >           // Setting the urlBar value in some cases causes userTypedValue to
> >-          // become set because of oninput, so reset it to its old value.
> >-          browser.userTypedValue = userTypedValue;
> >+          // become set because of oninput, so reset it to null.
> >+          browser.userTypedValue = null;
> 
> Please see bug 397815 where I'm removing this.

Ok, I removed this part from the patch I have just attached, so that our patches don't bitrot each other.


> >Index: browser/base/content/tabbrowser.xml

> >-              if (this.mBrowser.userTypedClear > 0 && aRequest)
> >+              if (this.mBrowser.userTypedClear > 0)
> >                 this.mBrowser.userTypedValue = null;
> 
> What is this change about? Without aRequest, there's no loading, so the comment
> isn't accurate anymore.
> 

There's a loading without request for anchor scrolls, and we want the location bar to be updated in this case.
Comment 51 neil@parkwaycc.co.uk 2007-12-27 13:16:49 PST
(In reply to comment #49)
>Neil, do you know if there are cases where this code set the userTypedClear
>field to a non-zero value?
>-                  if (this.mBrowser.userTypedClear > 0)
>+                  if (this.mBrowser.userTypedClear > 1)
>+                    this.mBrowser.userTypedClear -= 2;
>+                  else if (this.mBrowser.userTypedClear > 0)
>                     this.mBrowser.userTypedClear--;
I know one case is when you initiate a load while one is already in progress; the new load increments userTypedClear to 3, and it needs to remain positive so that the subsequent location change will clear userTypedValue.
Comment 52 neil@parkwaycc.co.uk 2007-12-27 14:04:05 PST
(In reply to comment #51)
>I know one case is when you initiate a load while one is already in progress;
>the new load increments userTypedClear to 3, and it needs to remain positive so
>that the subsequent location change will clear userTypedValue.
Actually I don't remember whether it's the first or second load (or both) that is typed and needs to be cleared.
Comment 53 neil@parkwaycc.co.uk 2007-12-27 14:18:09 PST
Comment on attachment 294676 [details] [diff] [review]
updated Firefox patch (v5)

>-      // The document loaded correctly, clear the value if we should
>-      if (browser.userTypedClear > 0 && aRequest)
>-        browser.userTypedValue = null;

>-    // 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++;
>-

>-    // The document is done loading, we no longer want the
>-    // value cleared.
>-    if (getBrowser().userTypedClear > 0)
>-      getBrowser().userTypedClear--;
Interesting... these changes would probably work in SeaMonkey because its tabbrowser is always in tabbed mode and therefore does all of the heavy lifting, but you might find that if you turn off all tabbed browsing (if that's even possible in Firefox; I haven't read the code carefully) then usedTypedClear would stop working altogether.
Comment 54 Florian Quèze [:florian] [:flo] 2007-12-27 14:39:28 PST
(In reply to comment #53)
> you might find that if you turn off all tabbed browsing (if that's
> even possible in Firefox; I haven't read the code carefully) then
> usedTypedClear would stop working altogether.
> 

When I have a single tab (and then, no visible tab bar), the userTypedClear value is still changed from tabbrowser.xml.  I don't see how I could 'turn off' tabbed browsing (I guess you mean using only browser.xml and not tabbrowser.xml) in Firefox.
Comment 55 neil@parkwaycc.co.uk 2007-12-27 14:48:59 PST
(In reply to comment #54)
>I don't see how I could 'turn off' tabbed browsing
In Mozilla 1.7 with default preferences, tabbed browsing wasn't enabled, and none of the tabbrowser goodies e.g. favicons were hooked up. In particular no tab progress listeners had been created that could change userTypedClear, which is why it had to be done in nsBrowserStatusHandler.js as well.
Comment 56 Dão Gottwald [:dao] 2007-12-27 14:57:12 PST
(In reply to comment #50)
> > >             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;
> > 
> > What is this change about? Without aRequest, there's no loading, so the comment
> > isn't accurate anymore.
> > 
> 
> There's a loading without request for anchor scrolls, and we want the location
> bar to be updated in this case.

That's not really "loading". Also, onLocationChange is called without aRequest when you switch tabs. |&& aRequest| was added for that reason in bug 231393 / bug 316059.
Comment 57 Florian Quèze [:florian] [:flo] 2007-12-27 15:28:22 PST
(In reply to comment #56)

> onLocationChange is called without aRequest when you switch tabs.
> |&& aRequest| was added for that reason in bug 231393 / bug 316059.
>

I guess that's why in attachment 235957 [details] [diff] [review] Gavin added back the hack in tabbrowser/updateCurrentBrowser.
Comment 58 neil@parkwaycc.co.uk 2007-12-27 16:05:49 PST
(In reply to comment #56)
>Also, onLocationChange is called without aRequest when you switch tabs.
Surely that only applies to the consumers of tabbrowser - its progress listener's onLocationChange always has a request.
Comment 59 neil@parkwaycc.co.uk 2007-12-27 16:09:35 PST
Comment on attachment 294676 [details] [diff] [review]
updated Firefox patch (v5)

>+            // Remember the current clear state, then set it to false
>+            // so we don't clear the userTypedValue when just switching
>+            // tabs. Set it back to its old state after we're done.
>+            var userTypedClear = this.mCurrentBrowser.userTypedClear;
>+            this.mCurrentBrowser.userTypedClear = 0;
So, given that you're removing the userTypedClear code from browser.js it would seem that you don't need this code.
Comment 60 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-01-08 12:53:38 PST
Comment on attachment 294676 [details] [diff] [review]
updated Firefox patch (v5)

Would be awesome to get some tests in for this... I guess that might not be very easy.
Comment 61 Florian Quèze [:florian] [:flo] 2008-01-08 15:29:24 PST
Created attachment 296041 [details] [diff] [review]
patch v6 (for checkin)

Comment 59 addressed. Ready for checkin.
Comment 62 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-01-08 20:08:43 PST
mozilla/browser/base/content/browser.js 1.923
mozilla/browser/base/content/tabbrowser.xml 1.257
mozilla/toolkit/content/widgets/browser.xml 1.116
Comment 63 Reed Loden [:reed] (use needinfo?) 2008-01-08 20:15:04 PST
Mano, Florian has commit privileges, fyi... :)
Comment 64 Rostislav Hristov 2008-01-10 11:24:20 PST
Works great here (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008010904 Minefield/3.0b3pre). Thanks!
Comment 65 Carsten Book [:Tomcat] - PTO-back Sept 4th 2008-02-06 05:08:47 PST
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008020304 Minefield/3.0b3pre ID:2008020304
Comment 66 Rostislav Hristov 2008-05-22 10:14:28 PDT
It was brought to my attention that a very similar problem still exists in Firefox 3 RC1. Every step for the reproduction is the same except the part that the user has to hit Enter.

STEPS TO REPRODUCE:

1)  Load https://bugzilla.mozilla.org/show_bug.cgi?id=213946
2)  Click in the URL bar, add "#c29" to the end of the URI and do not hit Enter, just leave the bar
3)  Click on the link that says #c30

EXPECTED RESULTS:  URL bar says
https://bugzilla.mozilla.org/show_bug.cgi?id=213946#c30

ACTUAL RESULTS:  URL bar says https://bugzilla.mozilla.org/show_bug.cgi?id=213946#c29
Comment 67 Boris Zbarsky [:bz] 2008-05-22 12:40:20 PDT
If the user hasn't hit enter, the page navigation should NOT change the url bar.  Otherwise the page could keep a user from typing in a URL.
Comment 68 Rostislav Hristov 2008-05-22 12:59:17 PDT
Boris,
where is that pattern described? Why IE, Safari and Opera don't work like this?
Can you give a sample of a site that keeps the user from typing in a URL.
Comment 69 Boris Zbarsky [:bz] 2008-05-22 13:57:30 PDT
> where is that pattern described? 

Other than in the code?  I have no idea.

> Why IE, Safari and Opera don't work like this?

Because they don't want to?  It's up to their UI designers to decide how their UI will behave.

> Can you give a sample of a site that keeps the user from typing in a URL

Any site that keeps setting location.hash every 50ms, say, if every set will reset the URL bar as you propose.
Comment 70 Kevin Newman 2008-05-22 14:12:57 PDT
The above couple of comments look like miscommunication.

To rephrase slightly, should it work like this:

If the location bar has focus, then scripts should not be able to update the location bar. If it doesn't scripts should be allowed to update the url.

Is it not that simple?
Comment 71 Boris Zbarsky [:bz] 2008-05-22 14:19:20 PDT
Just because it doesn't have focus doesn't mean I'm done typing the URL.  I could be copying some text to paste into it.  I could have just focused another window temporarily.
Comment 72 Boris Zbarsky [:bz] 2008-05-22 14:20:12 PDT
In any case, if what you want is a change in designed behavior, you probably want to file a separate bug.  This bug was about a situation in which behavior departed from the plan, whereas you want a change of plan.
Comment 73 neil@parkwaycc.co.uk 2008-05-22 14:26:01 PDT
(In reply to comment #68)
> Why IE, Safari and Opera don't work like this?
Worse still, IE does this even when I'm typing in its open location dialog, which makes no sense at all!
Comment 74 Rostislav Hristov 2008-05-23 07:38:52 PDT
Take for example Gmail which uses JavaScript to update the location.hash property. When the address bar value is edited like this the application cannot update it anymore and copying the address at a later stage won't be correct.

Why this behavior is limited to location.hash but not to location.href? This actually confuses Ajax/Flash linking but there are no such limitations for plain non-anchor links.

If you're convinced that the bug case is valid I'll create a new entry.
Comment 75 Mark Ross 2008-05-26 16:09:06 PDT
(In reply to comment #71)
> Just because it doesn't have focus doesn't mean I'm done typing the URL.  I
> could be copying some text to paste into it.  I could have just focused another
> window temporarily.
> 

Yeah, this is true, but if you start navigating again, in the same Firefox window, shouldn't the address bar lose focus automatically?

It's clear that you are no longer interested in whatever editing of the address bar you were doing at that point, no?
Comment 76 Rob Kinyon 2008-05-26 16:48:40 PDT
The original report of the bug was due to a RIA that allowed editing of the URI. One of our use cases was edit something, do something else, then edit again, then hit return. Our view was that the URI field should behave as a textbox that is the single element in a form whose action is location.href=location.href. So, forms generally don't submit on the leave event.
Comment 77 Mark Ross 2008-05-26 17:15:59 PDT
(In reply to comment #76)
> One of our use cases was edit something, do something else, then edit
> again, then hit return. 
> 

And that's a great goal, because IE totally screws the user on that front.

However, when the "do something else" in your use case, is to take steps such as clicking navigation links in the browser, I think that's an ideal time to start updating the address bar again.  Isn't that also a valid use case?

Comment 78 Rob Kinyon 2008-05-26 17:38:04 PDT
(In reply to comment #77)
> However, when the "do something else" in your use case, is to take steps such
> as clicking navigation links in the browser, I think that's an ideal time to
> start updating the address bar again.  Isn't that also a valid use case?
> 

That was another use-case. Our expectation was if the JS wanted to write to the URI, it should take precedence over any unused modifications.

Of course, the (now defunct) RIA that generated this bug report would have also accepted that the URI bar acted as a textbox that linked onBlur to onSubmit.
Comment 79 Kevin Newman 2008-05-27 08:56:55 PDT
If a script attempts to change the location bar while the user has focus, then it should not start getting updated again.

The user's focus in the location bar is always the most important. If the user clicks another UI element, or otherwise changes that focus (submits a form or something manually - which would require them to move focus from the location bar to something else, I'd imagine), then the url bar should be updated again - unless the location text has been changed, and the enter button hasn't been pressed because of the point made in Comment #71.

If that's not robust enough (only time will tell) then that's for another bug.
Comment 80 Mark Ross 2008-05-27 09:53:36 PDT
Ok, I think we can all agree, that if the user has started changing stuff in the address bar, that scripts shouldn't be allowed to overwrite that, until the user presses either ENTER (submit) or ESCAPE (cancel).

However, IMHO the statements made in Comment #71 do not apply, as in my suggested use case the user is clearly no longer interested in whatever partial updates they did to the address bar.

If the user starts to change the address bar, hits neither ENTER or ESCAPE, but starts navigating again in the same Firefox window/tab, that (in my mind) is the same as if they pressed ESCAPE.  At the point they start navigating again, the address bar should lose focus.  As an end user this seems pretty clear cut?

Comment 81 Rob Kinyon 2008-05-27 09:57:15 PDT
(In reply to comment #80)
> Ok, I think we can all agree, that if the user has started changing stuff in
> the address bar, that scripts shouldn't be allowed to overwrite that, until the
> user presses either ENTER (submit) or ESCAPE (cancel).
> 
> However, IMHO the statements made in Comment #71 do not apply, as in my
> suggested use case the user is clearly no longer interested in whatever partial
> updates they did to the address bar.
> 
> If the user starts to change the address bar, hits neither ENTER or ESCAPE, but
> starts navigating again in the same Firefox window/tab, that (in my mind) is
> the same as if they pressed ESCAPE.  At the point they start navigating again,
> the address bar should lose focus.  As an end user this seems pretty clear cut?
> 

In other words, onBlur should be treated as ESCAPE. That sounds reasonable to me.

Comment 82 Kevin Newman 2008-05-27 11:10:04 PDT
> In other words, onBlur should be treated as ESCAPE. That sounds reasonable to
me.

Just to clarify, onBlur would be treated as ESCAPE only if the text has not been edited (if the user pastes one thing in, and goes to copy another part, the onBlur would be triggered).
Comment 83 Boris Zbarsky [:bz] 2008-05-27 11:31:37 PDT
Can we please take the discussion to the appropriate forum?  In this case, this would be the mozilla.dev.apps.firefox newsgroup.  This bug, as I filed it, is fixed, and the mail from it is interfering with work actually getting done, to be honest...
Comment 84 Boris Zbarsky [:bz] 2008-05-27 11:32:10 PDT
Oh, and quoting the entirety of a comment in bugzilla is pretty poor form...
Comment 85 Rostislav Hristov 2008-05-27 12:23:55 PDT
I don't know what exactly we can discuss anymore. Here is the new bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=435932

Note You need to log in before you can comment on or make changes to this bug.