Location bar has wrong URL after 302 redirect for link opened in a new tab

RESOLVED FIXED

Status

SeaMonkey
UI Design
--
major
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: kmike, Assigned: neil@parkwaycc.co.uk)

Tracking

({fixed-seamonkey1.0, fixed-seamonkey1.1a, regression})

Trunk
fixed-seamonkey1.0, fixed-seamonkey1.1a, regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [verified-seamonkey1.1a], URL)

Attachments

(1 attachment, 1 obsolete attachment)

1.29 KB, patch
Darin Fisher
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 1

12 years ago
After some more investigation, the culprit is found: XUL.mfasl. After deleting
it this bug is WFM.

Status: UNCONFIRMED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 2

12 years ago
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.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
Summary: Location bar doesn't update URL after 302 redirect → Location bar has wrong URL after 302 redirect for link opened in a new tab
(Reporter)

Comment 3

12 years ago
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".
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

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
Confirming based on comments 4 and 5. Adding regression keyword. Changing
hardware/OS to all.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Linux → All
Hardware: PC → All

Updated

12 years ago
Depends on: 302575
@@ -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.

Updated

12 years ago
Version: unspecified → Trunk
(Assignee)

Comment 8

12 years ago
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
Assignee: guifeatures → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #194782 - Flags: superreview?(darin)
Attachment #194782 - Flags: review?(bzbarsky)
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

12 years ago
Comment on attachment 194782 [details] [diff] [review]
Proposed patch

r=bzbarsky, I guess, but please document this very clearly!
Attachment #194782 - Flags: review?(bzbarsky) → review+

Comment 11

12 years ago
Comment on attachment 194782 [details] [diff] [review]
Proposed patch

Please document the heck out of this.  sr=darin
Attachment #194782 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 12

12 years ago
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

12 years ago
cannot reproduce with SeaMonkey/2005092105-trunk/WinXP.
wfm/fixed?

Comment 14

12 years ago
s/noticable/noticeable/ ;-)
(Assignee)

Comment 15

12 years ago
Comment checked in with two other typos fixed too.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 16

12 years ago
SeaMonkey-only portion of patch checked in to the 1.8 branch.
Whiteboard: fixed-seamonkey1.0
Keywords: fixed-seamonkey1.0
Whiteboard: fixed-seamonkey1.0
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.
Attachment #222627 - Flags: superreview?(neil)
Attachment #222627 - Flags: review?(neil)
Attachment #222627 - Flags: approval-seamonkey1.1a?
(Assignee)

Comment 18

11 years ago
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.
Attachment #222627 - Flags: superreview?(neil)
Attachment #222627 - Flags: review?(neil)
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...
Attachment #222627 - Flags: review?(jag)

Comment 20

11 years ago
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 ...)
Attachment #222627 - Flags: review?(jag) → review+
Comment on attachment 222627 [details] [diff] [review]
(Bv1-181) <tabbrowser.xml>
[Checkin: Comment 22]

a=me for 1.1a
Attachment #222627 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
(Assignee)

Updated

11 years ago
Keywords: fixed-seamonkey1.1a
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
}
Attachment #222627 - Attachment description: (Bv1-181) <tabbrowser.xml> → (Bv1-181) <tabbrowser.xml> [Checkin: Comment 22]
Attachment #222627 - Attachment is obsolete: true
[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)
Whiteboard: [verified-seamonkey1.1a]

Updated

9 years ago
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.