Last Comment Bug 306810 - Location bar has wrong URL after 302 redirect for link opened in a new tab
: Location bar has wrong URL after 302 redirect for link opened in a new tab
Status: RESOLVED FIXED
[verified-seamonkey1.1a]
: fixed-seamonkey1.0, fixed-seamonkey1.1a, regression
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- major (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
http://www.google.com/search?hl=en&ie...
Depends on: 302575
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-02 01:13 PDT by kmike
Modified: 2008-07-31 04:22 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (1.29 KB, patch)
2005-09-03 12:41 PDT, neil@parkwaycc.co.uk
bzbarsky: review+
darin.moz: superreview+
Details | Diff | Review
(Bv1-181) <tabbrowser.xml> [Checkin: Comment 22] (1.21 KB, patch)
2006-05-19 08:13 PDT, Serge Gautherie (:sgautherie)
jag-mozilla: review+
csthomas: approval‑seamonkey1.1a+
Details | Diff | Review

Description kmike 2005-09-02 01:13:05 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050902 SeaMonkey/1.1a
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050902 SeaMonkey/1.1a

After clicking on links which redirect you to other pages with HTTP 302 Found,
URL bar contains the previous page URL instead of new one. Found it on google
results page - they sometimes contain redirects to the actual results.

Reproducible: Always

Steps to Reproduce:
1. Click on
http://www.speedtrader.com/dirinc/click.php?url=http://www.commodity-trading-software.com&id=746


Actual Results:  
URL bar contains
http://www.speedtrader.com/dirinc/click.php?url=http://www.commodity-trading-software.com&id=746


Expected Results:  
URL bar contains http://www.commodity-trading-software.com/

Note that a page info (ctrl-I) shows a right URL, not a previous one, so it is a
location bar which is confused about current URL.

My previous Linux build from 2005-07-13 doesn't exhibit this problem.

This may have something to do with bug 302575, though I'm not sure.
Comment 1 kmike 2005-09-02 01:44:33 PDT
After some more investigation, the culprit is found: XUL.mfasl. After deleting
it this bug is WFM.

Comment 2 kmike 2005-09-02 04:49:17 PDT
Ack, it isn't WFM, even in a pristine new profile. Updating summary and URL,
here're steps to reproduce:

1. Do a google search for "bookmarklets":
http://www.google.com/search?hl=en&ie=UTF-8&q=bookmarklets&btnG=Google+Search
2. Right click on a first result ("Bookmarklets Home Page - free tools for power
surfing"), select "Open link in a new tab"
3. Switch to a new tab, if you don't have it already focused
4. Observe 
"http://www.google.com/url?sa=t&ct=res&cd=1&url=http%3A//www.bookmarklets.com/&ei=3TkYQ63zFp3O4QHH8J3SDA"
in the location bar

Expected results:
http://www.bookmarklets.com/ in the location bar.

Some notes: pressing "reload" will fix URL. Selecting "open in a new window" as
well as simple clicking on a link to open in the same window doesn't trigger the
bug, only "open link in a new tab" does.
Comment 3 kmike 2005-09-02 05:41:29 PDT
Downloaded 
ftp://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2005-09-01-09-trunk/seamonkey-1.1a.en-US.linux-i686.tar.gz,

it shows the same bug. Testcase #1 works as well when using "open in a new tab".
Comment 4 Bruno 'Aqualon' Escherl 2005-09-02 09:12:50 PDT
Confirming the bug with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1)
Gecko/20050902 SeaMonkey/1.1a and Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.9a1) Gecko/20050901 SeaMonkey/1.1a

No problem with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1)
Gecko/20050901 Firefox/1.6a1 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20050902 Firefox/1.6a1

Seems the refresh of the location bar is forgotten after redirect.

Bruno

Comment 5 Bruno 'Aqualon' Escherl 2005-09-02 09:54:53 PDT
It's a regression (can someone set the keyword please?) between Mozilla/5.0
(Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050831 SeaMonkey/1.1a and
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050901
SeaMonkey/1.1a

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-08-31+10%3A09%3A00&maxdate=2005-09-01+09%3A20%3A00&cvsroot=%2Fcvsroot

Probably regression from the fix for bug 302575.

Bruno
Comment 6 Samuel Sidler (old account; do not CC) 2005-09-02 10:06:54 PDT
Confirming based on comments 4 and 5. Adding regression keyword. Changing
hardware/OS to all.
Comment 7 Bruno 'Aqualon' Escherl 2005-09-02 14:41:19 PDT
@@ -141,7 +155,13 @@
               catch (e) {
               }
             }
-            this.webNavigation.loadURI(aURI, aFlags, aReferrerURI, null, null);
+            try {
+              this.userTypedClear++;
+              this.webNavigation.loadURI(aURI, aFlags, aReferrerURI, null, null);
+            } finally {
+              if (this.userTypedClear)
+                this.userTypedClear--;
+            }
           ]]>
         </body>
       </method>

This part of the suite patch is the reason for the regression. Backing it out
makes it work, but causes Bug 302575 again.
Comment 8 neil@parkwaycc.co.uk 2005-09-03 12:41:36 PDT
Created attachment 194782 [details] [diff] [review]
Proposed patch

This problem basically affected loads that had to stop an existing load.
The stop wasn't originally a problem because userTypedClear was already 0.
Before: 0 (-1 stop) +1 from start = 1 therefore OK to clear
After: 0 +1 before loadURI -1 stop +1 start -1 after loadURI = 0 not OK
So to fix this I propose to make start/stop more important i.e. += 2.
This will only affect the way start/stop interacts with faked clear i.e.
New: 0 +1 before loadURI -1 (-2) stop +2 start -1 after loadURI = 1 so OK
Comment 9 Bruno 'Aqualon' Escherl 2005-09-04 08:32:09 PDT
I tested the patch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20050901 SeaMonkey/1.1a

This bug gets fixed and Bug 302575 keeps fixed too. Haven't seen any problem
with it so far.

Bruno
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-09-05 18:57:26 PDT
Comment on attachment 194782 [details] [diff] [review]
Proposed patch

r=bzbarsky, I guess, but please document this very clearly!
Comment 11 Darin Fisher 2005-09-06 16:37:49 PDT
Comment on attachment 194782 [details] [diff] [review]
Proposed patch

Please document the heck out of this.  sr=darin
Comment 12 neil@parkwaycc.co.uk 2005-09-16 05:14:26 PDT
Draft comment:

<!--
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 tempoarily 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 noticable 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
  interruptes the default load of about:blank.
-->
<field name="userTypedClear">
  1
</field>
Comment 13 baffclan 2005-09-22 07:40:24 PDT
cannot reproduce with SeaMonkey/2005092105-trunk/WinXP.
wfm/fixed?
Comment 14 Darin Fisher 2005-09-22 11:49:23 PDT
s/noticable/noticeable/ ;-)
Comment 15 neil@parkwaycc.co.uk 2005-09-22 12:08:36 PDT
Comment checked in with two other typos fixed too.
Comment 16 neil@parkwaycc.co.uk 2005-12-14 16:37:05 PST
SeaMonkey-only portion of patch checked in to the 1.8 branch.
Comment 17 Serge Gautherie (:sgautherie) 2006-05-19 08:13:02 PDT
Created attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

'approval-seamonkey1.1a=?':
Attachment 194782 [details] [diff] landed on (Trunk and) MOZILLA_1_8_BRANCH,
but this simple enhanced comment never made it to the branch: synchronizing.
Comment 18 neil@parkwaycc.co.uk 2006-05-19 08:16:18 PDT
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

Personally I don't think it's worth synchronising a comment, but you could always ask jag for a second opinion.
Comment 19 Serge Gautherie (:sgautherie) 2006-05-19 08:22:20 PDT
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

(In reply to comment #18)
> (From update of attachment 222627 [details] [diff] [review] [edit])
> Personally I don't think it's worth synchronising a comment, but you could
> always ask jag for a second opinion.

Not mandatory, for sure;
but I thought it would be good to sync', considering the trunk migration to the Toolkit file...
Comment 20 jag (Peter Annema) 2006-05-19 20:03:50 PDT
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

I say let's check this in. Cost to us is virtually nothing (right?) and it might clear this up to someone who's only got access to the 1.8 branch source (unlikely, but ...)
Comment 21 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-06-23 12:53:45 PDT
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

a=me for 1.1a
Comment 22 Serge Gautherie (:sgautherie) 2006-06-25 15:17:54 PDT
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]


Checkin: {
2006-06-23 14:26	neil%parkwaycc.co.uk 	mozilla/xpfe/global/resources/content/bindings/tabbrowser.xml 	1.126.2.27 	MOZILLA_1_8_BRANCH
}
Comment 23 Serge Gautherie (:sgautherie) 2006-06-25 15:20:29 PDT
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a3) Gecko/20060623 SeaMonkey/1.1a] (nightly) (W98SE)

V.Fixed on MOZILLA_1_8_BRANCH, between these two builds.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a3) Gecko/20060624 SeaMonkey/1.1a] (nightly) (W98SE)

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