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

RESOLVED FIXED

Status

()

Firefox
Toolbars and Customization
P4
normal
RESOLVED FIXED
14 years ago
3 years ago

People

(Reporter: paxunix, Assigned: Seairth Jacobs)

Tracking

Trunk
Points:
---
Bug Flags:
blocking-aviary1.0 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

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

14 years ago
Confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reassigning for Ben's consideration.

/be
Assignee: hyatt → bugs

Comment 3

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

Comment 4

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

Comment 5

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

Comment 6

13 years ago
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
Flags: blocking-aviary1.0?

Updated

13 years ago
Flags: blocking-aviary1.0? → blocking-aviary1.0+
Priority: -- → P4
Assignee: bugs → sspitzer
Created attachment 155248 [details] [diff] [review]
patch
Attachment #155248 - Flags: review?(bugs)

Comment 8

13 years ago
[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

13 years ago
p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-

Comment 10

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

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

13 years ago
(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

13 years ago
(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

13 years ago
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
Per comment 9, renominating.

/be
Flags: blocking-aviary1.0- → blocking-aviary1.0?
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
Also, use a space between ) and { in

              this.mCurrentBrowser.userTypedClear == false){

and all similar such constructs.

/be
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

13 years ago
not going to block on this.
Flags: blocking-aviary1.0? → blocking-aviary1.0-

Comment 20

13 years ago
(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.
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
Assignee: sspitzer → bugs

Comment 22

13 years ago
Created attachment 165856 [details] [diff] [review]
Adding handleURLBarRevert()

As applied to 2004-11-13 nightly.

Comment 23

13 years ago
(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
*** Bug 271963 has been marked as a duplicate of this bug. ***
*** Bug 273920 has been marked as a duplicate of this bug. ***
*** Bug 276529 has been marked as a duplicate of this bug. ***

Comment 27

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

12 years ago
Can this please be fixed before releasing 1.1?

Comment 29

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

*** Bug 295414 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1?
(Assignee)

Comment 31

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

12 years ago
Created attachment 185097 [details] [diff] [review]
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.

Updated

12 years ago
Attachment #165856 - Attachment is obsolete: true
Attachment #155248 - Attachment is obsolete: true
Attachment #155248 - Flags: review?(bugs)
Assignee: bugs → nobody
OS: Windows 2000 → All
QA Contact: bugzilla → toolbars
Hardware: PC → All
Version: unspecified → Trunk

Comment 33

12 years ago
(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!
(Assignee)

Comment 34

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

Updated

12 years ago
Attachment #185097 - Flags: review?(mconnor)
(Assignee)

Comment 35

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

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

New patch coming up later today.
Attachment #185097 - Attachment is obsolete: true
Attachment #185097 - Flags: review?(mconnor)

Comment 37

12 years ago
Created attachment 185890 [details] [diff] [review]
Patch v2 (Thanks to Seairth Jacobs for figuring out how to fix this)

Patch updated to comment #35.

Updated

12 years ago
Attachment #185890 - Flags: review?(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.
Attachment #185890 - Flags: review?(mconnor) → review+

Updated

12 years ago
Attachment #185890 - Flags: approval-aviary1.1a2?

Updated

12 years ago
Attachment #185890 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Whiteboard: [checkin needed]

Comment 39

12 years ago
(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.
fix checked in
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: blocking-aviary1.1?
Whiteboard: [checkin needed]

Comment 41

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

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

Comment 43

12 years ago
*** Bug 298877 has been marked as a duplicate of this bug. ***
*** Bug 287996 has been marked as a duplicate of this bug. ***
Assignee: nobody → seairth
You need to log in before you can comment on or make changes to this bug.