Closed Bug 521828 Opened 11 years ago Closed 10 years ago
Search entries and urls are not saved to urlbar until page is loaded
Build Id: Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091012 Fennec/1.0a3 and Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091012 Fennec/1.0b5pre Steps to Reproduce: 1. Go to www.cnn.com 2. Before the page title is shown in the url bar, click on the url bar 1. Go to the url bar, and enter a search entry (I used bugzilla) 2. Click on the search button 3. Before the page title is shown in the url bar, click on the url bar 1. Go to www.ilikemyfacebooks.net (or any page that doesn't exist) 2. Before the "Page Load Error" is shown in the url bar, click on the url bar Actual Results: The url of the previous page is shown Expected Results: The url and/or search entry should be "saved" to the url bar. It's prevalence should not depend on when the page is loaded
Wow! Those are some serious steps to reproduce. Is this typical behavior? Yes, I'm asking you to defend the blocking? flag.
I've seen it, usually with the first set of steps to reproduce.
I've seen this too.
Mark, I flagged it as blocking to get some eyes on it. I think its particularly nasty behavior, but not blocking 1.0 as it doesn't completely break the browser even though it degrades user experience...which would make it as part of the 1.1 release. Stuart, what do you think?
This happen because the onLocationChange has not fired yet so the getDisplayURI return the previous URI instead of the new one (which is not really know at this time). This patch put the value entered by the user if the uri is not know when the user enter into "edit" mode of the location bar
Assignee: nobody → 21
Attachment #419292 - Flags: review?(mark.finkle)
Is this patch still valid since it's been in review for 3 months now?
patch is still fine. I am just not sure I like it. It seems like a band-aid on the problem.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I don't see this as fixed on build: Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:184.108.40.206pre) Gecko/20100401 Namoroka/3.6.3pre Fennec/1.1a2pre The search entries and urls are only saved to the urlbar once a server has responded back to the initial request (and the page title is loaded)
Status: RESOLVED → REOPENED
Flags: in-litmus? → in-litmus-
Resolution: FIXED → ---
This patch (attachment 419292 [details] [diff] [review]) also causes some new problems. For example, if you click the urlbar after the title appears but while the page is still loading, then the edit field will contain the title instead of the URL.
So, the original problem was that we were replacing the urlbar contents with browser.currentURI. Since currentURI is updated only after load, it is incorrect while the page is still loading. Next we tried grabbing the URL from BrowserUI._edit.value, but that doesn't always work because (A) _edit.value might contain the title instead, and (B) DOMTitleChanged fires before onLocationChange, so we still sometimes end up displaying the title of the new page and the URI of the old one. Here's my proposal for solving these problems. This patch gives each Tab a "loadingURI" field that contains the URI of the new page, and uses that instead of the edit value. It also adds a call to updateURI during the "network start" progress event when the urlbar is empty - just like desktop Firefox. (This part of the code will be cleaned up slightly by an upcoming patch for bug 557238.)
Comment on attachment 419292 [details] [diff] [review] Patch r+ for posterity
Attachment #419292 - Flags: review?(mark.finkle) → review+
Assignee: 21 → mbrubeck
Minor update based on feedback from mfinkle - remember to clear loadingURI when loading is done.
This patch uses the browser loadgroup to tell if the load requests are still active and uses the loadgroup default request to return the loading URI
Attachment #437754 - Flags: review?(mbrubeck)
Comment on attachment 437754 [details] [diff] [review] alternate patch using loadgroup switching to vivien since he'll be awake earlier
Attachment #437754 - Flags: review?(mbrubeck) → review?(21)
Comment on attachment 437754 [details] [diff] [review] alternate patch using loadgroup This is an inventive approach and I prefer this one to the previous because there is less code change. btw, I've been to bed just a few before your ping! :)
Attachment #437754 - Flags: review?(21) → review+
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Attachment #419292 - Attachment is obsolete: true
Matt found an edge case where the urlbar can be empty when a new tab is loading (examples are URL from command line or code that opens new tabs with preset URL). This patch should fix that. Desktop Firefox does something similar
Attachment #437838 - Flags: review?(mbrubeck)
Attachment #437838 - Flags: review?(mbrubeck) → review+
pushed early load fix: http://hg.mozilla.org/mobile-browser/rev/a427a2bd4bc0
I'm using build: Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:220.127.116.11pre) Gecko/20100412 Namoroka/3.6.4pre Fennec/1.1a2pre and I still get the previous/current page's url when I do any of the steps to perform on comment #0
I think this is expected. In desktop Firefox there is a (sometimes long) delay before the URL bar changes after clicking a link or performing a search. According to https://developer.mozilla.org/en/nsIWebProgressListener this is because we don't know right away whether a new request will replace the current document. (For example, it might be a download instead.) These patches should reduce the times when outdate URIs are displayed. In particular they should fix cases where an old URI is displayed in the urlbar *after* the new one has already been entered there (e.g. the third set of repro steps in comment 0). The most recent patch also fixes the regression in comment 10.
the responsiveness seems much snappier now with the patches and the testcase in comment 0. going to mark verified in Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:18.104.22.168) Gecko/20100603 Firefox/3.6.5pre Fennec/1.1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.