227826 - URL of current tab incorrectly changed when Alt+Enter opens new tab from location bar

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect, P4)

Description

[paxunix]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031208 Firebird/0.7+ (Oxs G7 SSE optimized)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031208 Firebird/0.7+ (Oxs G7 SSE optimized)

After typing a URL into the location bar and pressing Alt+Enter to open that URL
in a new tab, the tab that was active when pressing Alt+Enter has its location
bar URL incorrectly set to the typed value.  Note that the new tab is opened
with the correct URL and its location bar behavior seems normal.

Reproducible: Always

Steps to Reproduce:
1.  Go to some website (URL-a) in a given tab (Tab-a).
2.  Type a new URL (URL-b) into the location bar and press Alt+Enter to open it
in a new tab.

Actual Results:  
Tab-a has had its URL-a replaced with URL-b in the location bar.
Tab-b correctly has URL-b in the location bar.

Expected Results:  
Tab-a should still have URL-a in the location bar.

Note that this behavior makes sense if Tab-a is a new, empty tab.  This is most
likely a regression due to the fix for bug 203102.

I realize that you can hit Esc within the location bar when Tab-a is active and
URL-a will be restored.  It is incorrect, IMO, since I'm not changing the "URL"
that I want associated with that tab (e.g. for a bookmarklet or something), but
am creating a new tab entirely.  If I wasn't opening a new tab, I would expect
Tab-a to have whatever I've typed in the location bar.

Comment 1

[Simon Paquet [:sipaq]]

Confirming.

Comment 2

[Brendan Eich [:brendan]]

Reassigning for Ben's consideration.

/be

Comment 3

[Simon Paquet [:sipaq]]

*** Bug 229509 has been marked as a duplicate of this bug. ***

Comment 4

[Christian Weyland]

*** Bug 238476 has been marked as a duplicate of this bug. ***

Comment 5

[José Jeria]

*** Bug 246136 has been marked as a duplicate of this bug. ***

Comment 6

[Christian Weyland]

This bug can be *very* irritating and might also lead to serious situations,
i.e. when I work on different webservers (development/production) and do some
changes on one of them.

I once was wondering why those changes wouldn't show up in my browser when
refreshing. Later, I noticed that FF was just showing the productions server's
URL while having loaded the dev. server's website.

requesting blocking-aviary1.0

Comment 7

[Nian Liu(n/a in a long time)]

Attachment: patch

Comment 8

[Ingmar]

[Disclaimer: This comment is not worth reading if you are only interested in the
technical aspects of the bug. It's just for pointing out the importance of this
bug.]

Some points why I think this is a bug that should be fixed ASAP:
1.) It can be very confusing when working with different versions of the same site
This is the same as Christian oulined in comment #6. 
Example: I'm working a lot with a web based CMS, and often working on multiple
instances of the CMS interface at the same time, and it can sometimes cause real
harm if you are tricked by the wrong information of the location bar into
thinking you were working on system A although it is actually system B.

2.) This bug is really ugly because you notice it ALWAYS when working with
ALT+ENTER for opening a tag. 
Example: It looks strange having cnet.com in the location bar while reading a
slashdot.org page. Smells like a buggy browser.

3.) As told above in the slashdot.org/cnet.com example, it is not so much of a
problem, because you immediately notice that there's a wrong URL in the location
bar.
However, in systems like bugzilla where just a URL prefix like ?id=227826
distinguishes between different pages, you often don't notice the wrong URL.
Example: I have multiple tabs with bug reports concurrently opened. Now I want
to paste the URL of a bug into my ICQ instant messanger. The problem is, I can't
be sure that the URL in the location bar, is really the URL of the bug I'm
looking at. That means I might paste a wrong URL into my ICQ.

None of these problems are invented, I have experienced ALL of them.

Please fix this before releasing 1.0 final.

Comment 9

[Asa Dotzler [:asa]]

p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 

Comment 10

[Uriah Narmed]

For what it's worth, I was able to fix this by adding

handleURLBarRevert();

after

var t = gBrowser.addTab(url, aPostData); // open link in new tab

in BrowserLoadURL in browser.js.

Comment 11

[Darnell]

Was the fix proposed by "Uriah Narmed" ever tested?  It would be good to fix
this before the final release of 1.0.

Comment 12

[Lisa]

(In reply to comment #11)
> Was the fix proposed by "Uriah Narmed" ever tested?  It would be good to fix
> this before the final release of 1.0.

I tried it on FF0.10, winXP Home, and it didn't work for me.

Comment 13

[Uriah Narmed]

(In reply to comment #12)
> I tried it on FF0.10, winXP Home, and it didn't work for me.

Just tried re-applying it to RC2 and it works correctly for me.  The diff of
browser.js:

1434a1435
>       handleURLBarRevert();

Comment 14

[Ingmar]

I was able to make it work in RC1 on Debian by inserting handleURLBarRevert();
after: 

var t = gBrowser.addTab(url, null, null, aPostData); // open link in new tab

Comment 15

[Brendan Eich [:brendan]]

Per comment 9, renominating.

/be

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 155248 [details] [diff] [review]
patch

Drive-by: don't make gratuitous (and deleterious) white space changes (removing
those blank lines).

Also, use // ... for comments, not //... (put a space after //).

Finally, you have clearly introduced a bug by doubling the : in about:blank
here:

+	     var blank = (aURI == "about::blank");

Getting ben to look at the substance of the patch, but please attach a revised
version that corrects the above flaws.

/be

Comment 17

[Brendan Eich [:brendan]]

Also, use a space between ) and { in

              this.mCurrentBrowser.userTypedClear == false){

and all similar such constructs.

/be

Comment 18

[Brendan Eich [:brendan]]

So I should read the bug's comments in order -- duh.  Anyway, can we have a
patch corresponding to the change described in comment 10, that multiple people
testify fixes the bug and introduces no others?  It's very unlikely that this
will make 1.0, but it definitely won't without a real patch that's been tested
and vouched for by trusted members of the community.

/be

Comment 19

[Asa Dotzler [:asa]]

not going to block on this.

Comment 20

[Lisa]

(In reply to comment #19)
> not going to block on this.

So when will it get in? Isn't one of the selling points of firefox that it has
tabbed browsing? So shouldn't it, by v1.0, have all extremely obvious and
annoying tabbed browsing bugs like this one eliminated? I know it's only a
cosmetic thing, but many people base their opinion of something heavily on
cosmetics.

Comment 21

[Brendan Eich [:brendan]]

No one attached a small, tested fix-patch here that didn't have issues.  Someone
do that, and we'll do our best to pick it up and put it in an update.

/be

Comment 22

[Uriah Narmed]

Attachment: Adding handleURLBarRevert()

As applied to 2004-11-13 nightly.

Comment 23

[Lisa]

(In reply to comment #22)
> Created an attachment (id=165856)
> Adding handleURLBarRevert()
> 
> As applied to 2004-11-13 nightly.

1) That patch no longer applies since the contents after "var t =
gBrowser.addTab(" have changed.

2) I tried adding handleURLBarRevert(); after the new "var t = ..." line as well
as other places in that function. None of them worked. The address bar never
reverted properly.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
WinXP home, SP1

Comment 24

[:Gavin Sharp [email: [e-mail]]]

*** Bug 271963 has been marked as a duplicate of this bug. ***

Comment 25

[Phil Ringnalda (:philor)]

*** Bug 273920 has been marked as a duplicate of this bug. ***

Comment 26

[:Gavin Sharp [email: [e-mail]]]

*** Bug 276529 has been marked as a duplicate of this bug. ***

Comment 27

[Ray Speth]

1) This bug is still present in the 1.01 release. Can someone please fix this?
2) Installing the "Tabbrowser Extensions" extension fixes this particular problem. 

Comment 28

[Darnell]

Can this please be fixed before releasing 1.1?

Comment 29

[Alan D Winter]

I am viewing this bug report. It is the only tabbed window currently open. Yet
the URL bar shows http://www.votemichaelhoward.com. Even pressing F5 on this bug
report window won't alter the title of the browser window to anything other than
"VoteMichaelHoward.com ? - No thanks says the team at - Mozilla Firefox "

Comment 30

[:Gavin Sharp [email: [e-mail]]]

*** Bug 295414 has been marked as a duplicate of this bug. ***

Comment 31

[Seairth Jacobs]

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8)
Gecko/20050511 Firefox/1.0.4

Like some, I could not get the handleURLBarRevert() funtion to work in
BrowserLoadURL().  As it turned out, I had an extension installed that overrides
BrowserLoadURL().  In my case, the extension is Tab Mix (0.2).  When I edit
tabmix.jar!content/tabmix/links/links.js (function TBP_BrowserLoadURL):

  464: theURI = theBar.value;
  465:
+ 466: handleURLBarRevert();
+ 467:
  468: __TBP_LoadBarURL(elementID, theBar, theURI, theEvent, theTabPref,
gPref.getBoolPref("browser.tabs.loadUrlInBackground"), thePostData);

Things work fine.  I then removed the extension and tried the standard
BowserLoadURL (browser.jar!content/browser/browser.js) with the following change:

  1426:  var url = gURLBar.value;
+ 1427:
+ 1428:  handleURLBarRevert();
+ 1429:
  1430:  if (url.match(/^view-source:/)) {

and again, it worked.

Frankly, this is such an quick and insignificant fix, it should be able to make
its way into 1.0.5.  I know an updated patch is needed, but I cannot create one
where I am, so I will leave that up to some other enterprising individual.  I
will also notify the developer of Tab Mix of the needed change.

Comment 32

[Robert Parenton]

Attachment: Add handleURLBarRevert() - Updated to tip

Seairth, can you test your change in a current trunk build?  If it works there,
I will request review on this patch.

Comment 33

[brian]

(In reply to comment #31)

> BrowserLoadURL().  In my case, the extension is Tab Mix (0.2).  When I edit
> tabmix.jar!content/tabmix/links/links.js (function TBP_BrowserLoadURL):

THANK YOU!

The above fix applies to TBP (tabbrowser preferences) extension as well, except
the file to patch is tabprefs.jar!content/tabprefs/tpscript/open.js, with the
same function name.  (I'm assuming that Tab Mix borrowed the TBP code, hence the
function name.)

After making the above change the problem is gone.  Hallelujah!

Comment 34

[Seairth Jacobs]

(In reply to comment #32)
> Created an attachment (id=185097) [edit]
> Add handleURLBarRevert() - Updated to tip
> 
> Seairth, can you test your change in a current trunk build?  If it works there,
> I will request review on this patch.

I downloaded and tested the patch on last night's build (June 1, Deer Park Alpha
1).  The change works.  I also tried re-installing Tab Mix 0.2.1, but it is not
compatible with the current build, so I cannot confirm that extension (and
likewise tabbar preferences).

Comment 35

[Seairth Jacobs]

It turns out that there is a slightly better way to do this:

  1542:function BrowserLoadURL(aTriggeringEvent, aPostData)
  1543:{
  1544:  var url = gURLBar.value;
  1545:  if (url.match(/^view-source:/)) {
+ 1546:    handleURLBarRevert();
  1547:    BrowserViewSourceOfURL(url.replace(/^view-source:/, ""), null, null);
  1548:  } else {
  1549:    if (gBrowser.localName == "tabbrowser" &&
  1550:        aTriggeringEvent && 'altKey' in aTriggeringEvent &&
  1551:        aTriggeringEvent.altKey) {
+ 1552:      handleURLBarRevert();
  1553:      content.focus();
  1554:      var t = gBrowser.addTab(url, null, null, aPostData); // open link
in new tab

The reason is that the prior suggestion I made causes the urlbar to revert even
if loading a link in the same window.  If a site responds quickly enough, you
don't notice it because the new URL overwrites the reverted URL.  However, if
there is a delay in the server response, the user sees the revert, which can be
confusing.

Note that I have placed the handleURLBarRevert() in two places.  The second
location handles the Alt+Enter (new tab) issue.  The first is included because
the "view-source:" pseudo-scheme causes a new window to open, leaving the
original window's urlbar with an incorrect value.

I have tested this change in both the 1.0.4 and the current trunk build (above
code from trunk, June 8 build).  I have not looked at the tabbar preferences or
tabmix, but they should be fairly similar.

Comment 36

[Robert Parenton]

Comment on attachment 185097 [details] [diff] [review]
Add handleURLBarRevert() - Updated to tip

New patch coming up later today.

Comment 37

[Robert Parenton]

Attachment: Patch v2 (Thanks to Seairth Jacobs for figuring out how to fix this)

Patch updated to comment #35.

Comment 38

[Mike Connor [:mconnor]]

Comment on attachment 185890 [details] [diff] [review]
Patch v2 (Thanks to Seairth Jacobs for figuring out how to fix this)

r=me, but there's another patch that may bitrot this about the view-source:
URLs forcing a new window.

Comment 39

[Robert Parenton]

(In reply to comment #38)
> (From update of attachment 185890 [details] [diff] [review] [edit])
> r=me, but there's another patch that may bitrot this about the view-source:
> URLs forcing a new window.

What bug is that patch in?  I'd like to try to keep it current until it gets
checked in.

Comment 40

[Daniel Brooks [:db48x]]

fix checked in

Comment 41

[mark]

BUT, when entering url like cnn and pressing ctrl+enter and quickily switch the
tab and switch back you will see the wrong address in address bar agian. It
shows cnn, not http://www.cnn.com. The ctrl+enter changes it.

1. Enter cnn to address bar and hit ctrl+enter
2. switch quickly the tabs
3. Addressbar is showing now the address without http://www.    .com

Comment 42

[Elmar Ludwig]

*** Bug 297771 has been marked as a duplicate of this bug. ***

Comment 43

[Elmar Ludwig]

*** Bug 298877 has been marked as a duplicate of this bug. ***

Comment 44

[Mike Connor [:mconnor]]

*** Bug 287996 has been marked as a duplicate of this bug. ***