Closed Bug 465673 Opened 16 years ago Closed 15 years ago

tabs opened from links should appear next to the current tab (instead of at the end of the tabstrip)

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3.7a1
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: ronin.achilles, Assigned: jmorkel)

References

(Blocks 1 open bug, )

Details

(Keywords: user-doc-complete, verified1.9.2, Whiteboard: [parity-IE][parity-Chrome][parity-Opera])

Attachments

(1 file, 12 obsolete files)

9.54 KB, patch
beltzner
: ui-review+
beltzner
: approval1.9.2+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1

This bug is based on Beltzner's comment in Bug #262459:

https://bugzilla.mozilla.org/show_bug.cgi?id=262459#c55

"Adding options is not something we should be doing lightly to Firefox. If we
believe that the default behaviour should be that new tabs should open adjacent
to the current tab, then let's change that default behaviour. My point in
comment 52 is that the WONTFIX is about adding an option, not changing the
default behaviour. I think there are reasonable arguments to re-evaluate our
"always at the end of the tabstrip" behaviour in a world where more users are
browsing with many tabs.

Right now that's being tracked in bug 161836, which is a Seamonkey bug, but we
could do with a Firefox one, as well."

In summary, what we intend to achieve with this bug is to change the default 'Tab Opening' position for Firefox.

My take on the expected new behavior is:
- Links from current tab -> Open next to Current Tab
- Links from external applications -> Open at the end of the Tab Bar
- Loaded Tabs (Alt+Enter in URL Bar) -> Open next to Current Tab
- Unloaded (New Tabs) -> Open at the end of the Tab Bar

In addition, when the links from current tab open next to the Current Tab, they should follow the order [1][A][B][C] instead of [1][C][B][A].

We can ofcourse look at other combinations of this behavior besides the one described above.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
This is the default behavior in windows.
But I think it might be better as an option, e.g. bug 262459.
Blocks: 262459
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3.1?
Martijn, if you see Beltzner's comment, thats exactly why Bug #262459 was WONTFIXED - i.e. we do not want to give this as an option. As per him, if this behavior (next to current tab) is more usable and desired, then that should be the default behavior; which I think is fine. It will definitely be better than the current default behavior.
Yes, I know, but I still want bug 262459 to be fixed, basically.
Attached patch IE7 behavior (obsolete) — Splinter Review
This is an unbitrotted version of my patch from bug 262459 comment #12. It implements IE7's tab opening behavior (can be disabled through a pref):

* Tabs opened from pages (i.e. those loading with a URL) are loaded right besides the currently selected tab.
* Exception: New blank tabs continue to be opened at the end of the tabstrip.
* Opening several tabs in a row will open these tabs in the same order as they were opened. I.e. opening three new tabs from tab A in [A B C] will result in [A 1 2 3 B C].
* "in a row" means: no other tab is selected and the current tab isn't moved around.
Attachment #348963 - Flags: ui-review?(beltzner)
OS: Windows Vista → All
Hardware: PC → All
Whiteboard: [parity-IE][parity-Chrome]
way out of scope for 3.1
Flags: blocking-firefox3.1? → blocking-firefox3.1-
Could you clarify?
(In reply to comment #6)
> way out of scope for 3.1

Disagree entirely here. I'll test this patch locally and get back to you on the ui-r in time for Beta 3. The heuristic being used is simple, understandable, and based on some early research from Patrick Dubroy from the University of Toronto, expected.
Flags: blocking-firefox3.1- → blocking-firefox3.1?
(not blocking, though)
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
beltzner: Ping? (ui-r needed RSN if this is to make Beta 3 code freeze)
Whiteboard: [parity-IE][parity-Chrome] → [parity-IE][parity-Chrome][parity-opera][needs ui-review beltzner]
Comment on attachment 348963 [details] [diff] [review]
IE7 behavior

Sorry, yes, I think we should try this. We should land it on mozilla-central first and blog about it before hand, though. Who's best to review?
Attachment #348963 - Flags: ui-review?(beltzner) → ui-review+
Attachment #348963 - Flags: review?(mconnor)
Attachment #348963 - Flags: review?(gavin.sharp)
Comment on attachment 348963 [details] [diff] [review]
IE7 behavior

Hoping that either Gavin or mconnor will find the time to review this simple patch.

In the meantime: Any volunteers for blogging? See comment #4 for the spec.
Assignee: nobody → zeniko
Whiteboard: [parity-IE][parity-Chrome][parity-opera][needs ui-review beltzner] → [needs review gavin/mconnor][parity-IE][parity-Chrome][parity-Opera]
I'll blog about this tomorrow. Looks like Ben beat me to the punch, somewhat: http://blog.chromium.org/2009/01/tabbed-browsing-in-google-chrome.html ;)
Comment to attachment patch from Simon Bünzli 

Where he says among other things:
**************************************************************************
* Tabs opened from pages (i.e. those loading with a URL) are loaded right
besides the currently selected tab.
* Exception: New blank tabs continue to be opened at the end of the tabstrip.
....
....
*************************************************

I really, really hope that this one:
* Exception: New blank tabs continue to be opened at the end of the tabstrip.

Could be changed in one way or another.

I often have many tabs in Firefox.
And if I have an active tab that is not the last one, and want to open a new empty tab then I would like this new tab to open next to my current one.

If this cannot be made default, then please make it an option in the
Tools->Options->Tabs area (end user options are not that bad).

I find current beaviour super annoying, and find myself often moving tabs because of how this works today.
(In reply to comment #15)
> I really, really hope that this one:
> * Exception: New blank tabs continue to be opened at the end of the tabstrip.
> 
> Could be changed in one way or another.
> 
> I often have many tabs in Firefox.
> And if I have an active tab that is not the last one, and want to open a new
> empty tab then I would like this new tab to open next to my current one.
> 
> If this cannot be made default, then please make it an option in the
> Tools->Options->Tabs area (end user options are not that bad).
> 
> I find current beaviour super annoying, and find myself often moving tabs
> because of how this works today.

I think this can be argued either way.  We'd have to look at the probability of a user opening a new blank tab to be used on the same topic of the current tab, and the probability of a user opening a new blank tab to be used on a new topic.  I agree it should be user configurable. I personally use an external link to open all my new firefox instances into a tab in the current window, so having them show up to the far right is greatly preferable.  However, I also open up new blank tabs from within the interface, and I would say that for that case, it's task dependent. For most cases, I will actually open a new window by topic, so I don't need the tabs to stay in any topic order, and instead, I more often want the tabs to be opened chronologically (to the far right). Finally, it would actually complicate things if I had to think about which one to do.
(In reply to comment #15)
> If this cannot be made default, then please make it an option

FWIW: Whatever will be chosen as the sane default, the other one will be a hidden option (just have a look at the start of the patch for the available options).
Status: NEW → ASSIGNED
Attached patch test (obsolete) — Splinter Review
Gavin, mconnor: Any estimate as to whether you're going to get to this before code freeze?
Attachment #356260 - Flags: review?(gavin.sharp)
Comment on attachment 348963 [details] [diff] [review]
IE7 behavior

>diff -r 0cd41f599080 browser/app/profile/firefox.js

>+// 0 = open new tabs at the end
>+// 1 = open new tabs to the immediate right
>+// 2 = open new tabs to the right (almost correct order)
>+// 3 = open new non-blank tabs to the immediate right
>+// 4 = open new non-blank tabs to the right (almost correct order)
>+pref("browser.tabs.tabOpenLogic", 4);

"almost correct order" means nothing to me. "to the right of any existing tabs opened from the current tab while it is selected" perhaps?

>diff -r 0cd41f599080 browser/base/content/tabbrowser.xml

>+            // values for tabOpenLogic:
>+            // 0 = open new tabs at the end
>+            // 1 = open new tabs to the immediate right
>+            // 2 = open new tabs to the right (almost correct order)
>+            // 3 = open new non-blank tabs to the immediate right
>+            // 4 = open new non-blank tabs to the right (almost correct order)
>+            let tabOpenLogic = this.mPrefs.getIntPref("browser.tabs.tabOpenLogic");
>+            if (tabOpenLogic > 0 && (!blank && aURI || tabOpenLogic < 3)) {

I think we should just change blank to be (!aURI || aURI == "about:blank"), which would allow you to simplify this check. In the aURI=="" case loadURIWithFlags just defaults to about:blank anyways. Can do that in a followup I guess if you want to minimize unrelated changes.

Re-getting the pref for each new tab will lead to strange behavior if you change from 2/4 to 1/3 without otherwise triggering a reset of _openedTabsCount. I don't think we really need to support this pref being "live", so the simplest fix is to only get this pref once in the constructor.

>           this._browsers = null; // invalidate cache
>+          if (this.mCurrentTab == aTab)
>+            // make sure not to open the next new tab in an unexpected position
>+            this._openedTabsCount = 0;
>+

nit: add a newline before this code you're adding, and use brackets since the comment makes it multi-line.

I think the idea from comment 0 of handling external tabs differently (always opening them at the end of the tabstrip) makes a lot of sense. We could do that relatively easily by having the Ci.nsIBrowserDOMWindow.OPEN_NEWTAB code in browser.js call moveTabTo. Beltzner, what do you think?
Attachment #348963 - Flags: review?(gavin.sharp) → review+
(In reply to comment #19)
> Re-getting the pref for each new tab will lead to strange behavior if you
> change from 2/4 to 1/3 without otherwise triggering a reset of
> _openedTabsCount. I don't think we really need to support this pref being
> "live", so the simplest fix is to only get this pref once in the constructor.

Having looked at the test now, I suppose this would make it more complicated. I guess you can just adjust tabbrowser's existing _prefObserver accordingly (have it cache the value, and clear _openedTabsCount when needed).
Attachment #356260 - Flags: review?(gavin.sharp) → review+
Comment on attachment 356260 [details] [diff] [review]
test

>diff --git a/browser/base/content/test/browser_tabOpenLogic.js b/browser/base/content/test/browser_tabOpenLogic.js

>+  // 3: all non-blank tabs are opened to the immediate right
>+  
>+  gPrefService.setIntPref("browser.tabs.tabOpenLogic", 3);
>+  tabs = tabOpenDance();
>+  
>+  is(tabs[1], gBrowser.mTabs[7], "tab was opened to the immediate right");
>+  is(tabs[2], gBrowser.mTabs[6], "next tab was opened to the immediate right");

The pref-changing problem is causing this test to pass when it shouldn't. This test should be:

is(tabs[1], gBrowser.mTabs[6], "tab was opened to the immediate right");
is(tabs[2], gBrowser.mTabs[5], "next tab was opened to the immediate right");

The last tab from the previous test leaves _openedTabsCount at 1, which means the second and third tabs from tabOpenDance() are opened after the first about:blank tab rather than before it.

r=me with that changed (and the test passing!)
Attached patch updated to comments (obsolete) — Splinter Review
(In reply to comment #19)
> "almost correct order" means nothing to me.

Fixed.

> I think we should just change blank to be (!aURI || aURI == "about:blank")

Fixed (was filed as bug 468053).

> Re-getting the pref for each new tab will lead to strange behavior if you
> change from 2/4 to 1/3 without otherwise triggering a reset of
> _openedTabsCount.

Fixed.

> nit: add a newline before this code you're adding, and use brackets since the
> comment makes it multi-line.

Fixed.

> I think the idea from comment 0 of handling external tabs differently (always
> opening them at the end of the tabstrip) makes a lot of sense.

We should already get this behavior per <http://hg.mozilla.org/mozilla-central/annotate/84508dcaf103/browser/base/content/browser.js#l4343> (where we open a blank tab for passed-in URLs first).

Finally, I've fixed an edge case where closing a background tab could lead to unexpected behavior due to _openedTabsCount not being correctly decremented.

Would you mind having another quick look at the changes before checking this in? Thanks!
Attachment #348963 - Attachment is obsolete: true
Attachment #356260 - Attachment is obsolete: true
Attachment #356407 - Flags: review?(gavin.sharp)
Attachment #348963 - Flags: review?(mconnor)
Whiteboard: [needs review gavin/mconnor][parity-IE][parity-Chrome][parity-Opera] → [needs review gavin][parity-IE][parity-Chrome][parity-Opera]
Attachment #356407 - Flags: review?(gavin.sharp) → review+
Comment on attachment 356407 [details] [diff] [review]
updated to comments

(In reply to comment #22)
> We should already get this behavior per
> <http://hg.mozilla.org/mozilla-central/annotate/84508dcaf103/browser/base/content/browser.js#l4343>
> (where we open a blank tab for passed-in URLs first).

Ah, right! We should add a comment there so that behavior isn't changed unintentionally. One of the first things I thought of when looking at that code is "why it goes through all that trouble to call loadURI directly on the window rather than using tabbrowser APIs?".

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml

nit: use mTabContainer internally.

>           pb2.addObserver("browser.tabs.closeButtons", 
>+                          this._prefObserver, false);
>+          pb2.addObserver("browser.tabs.tabOpenLogic", 
>                           this._prefObserver, false);

Don't want to scope creep this patch too much, so this will do for now, but really we should be using a single pref branch with an appropriate root rather than the root branch, and a single observer. Can you get a bug on file on cleaning this up?
Attached patch for check-in (obsolete) — Splinter Review
(In reply to comment #23)
> We should add a comment there so that behavior isn't changed unintentionally.

Done.

> nit: use mTabContainer internally.

Fixed.

> we should be using a single pref branch with an appropriate root

Filed bug 473065.
Attachment #356407 - Attachment is obsolete: true
Attachment #356414 - Flags: approval1.9.1?
Keywords: checkin-needed
Whiteboard: [needs review gavin][parity-IE][parity-Chrome][parity-Opera] → [parity-IE][parity-Chrome][parity-Opera]
Target Milestone: --- → Firefox 3.2a1
re: comment 15 and comment 16

FWIW, it's not a matter of probability. It's a matter of looking at use cases, and only trying to be clever about the user's intent when we can accurately predict it.

When a web page A contains a link to another page B and specifies that it is to be opened in a new window, we can assume that A and B are associated in the user's mind and cognitive model of where her tabs came from.

When a new tab is opened, it sometimes has to do with the page the user is viewing, and other times represents an entirely new task. Until we come up with a better mechanism for accurately understanding which is which, we shouldn't make assumptions and always act in a consistent manner.
Keywords: checkin-needed
Whiteboard: [parity-IE][parity-Chrome][parity-Opera] → [needs review gavin/mconnor][parity-IE][parity-Chrome][parity-Opera]
Target Milestone: Firefox 3.2a1 → ---
Waiting for this to land on trunk before approving ...
Whiteboard: [needs review gavin/mconnor][parity-IE][parity-Chrome][parity-Opera] → [parity-IE][parity-Chrome][parity-Opera]
Keywords: checkin-needed
As promised, I've written a post to Planet Mozilla (http://www.beltzner.ca/mike/archives/2009/01/a-small-change.html) to give people a heads-up about this upcoming change.
Keywords: late-compat
Its really great to see this some into play.

Although, I personally think that the only time tabs should be opened on the far right is when an external application requests a tab to be opened (e.g. from a mail client)... when I open a new tab (new tab button or keyboard short-cut), its more often than not related to the current group of tabs (action)... for example, if I see a word that I want to look up on wikipedia.

Would this be any chance be configurable in the "about:config"?

As to the idea where opening multiple tabs from a single page, and them being stacked... yes, that is a very nice feature.
(In reply to comment #28)
> Its really great to see this some into play.
> 
> Although, I personally think that the only time tabs should be opened on the
> far right is when an external application requests a tab to be opened (e.g.
> from a mail client)... when I open a new tab (new tab button or keyboard
> short-cut), its more often than not related to the current group of tabs
> (action)... for example, if I see a word that I want to look up on wikipedia.
> 
> Would this be any chance be configurable in the "about:config"?
> 
> As to the idea where opening multiple tabs from a single page, and them being
> stacked... yes, that is a very nice feature.

Agreed that external application requests should be opened to the far right.  Furthermore, I think that new firefox.exe executions after one is already running should also be configurable to open up as a new tab, for a truly single window mode, and this should ALSO be opened to the far right.

However, though that feature isn't as of yet a currently available option, my point is that if we foresee Firefox eventually adding the feature, to allow for a truly single window mode, the user could conveniently get into the habit of clicking the new tab button when wanting to "insert" a new tab for a related topic, like you want, but then executing firefox.exe for any new topics, which would also open in a new tab, but to the far right, like I want, satisfying both of us.  Sure, single window mode won't be the default, as a new window is probably what most users expect, but the aforementioned feature should at least be available to potentiate a truly single window mode for users who want it. This, again, doesn't have so much immediate bearing on this particular bug in practice, but in planning for the future, it does. 

Furthermore, like you said, just practically speaking, external requests should open to the far right, the most common example being links in mail clients. I actually even take advantage of this external request behavior to hack into Firefox the behavior I describe above. I set Firefox to single window mode, and I have my desktop and Quick Launch "Firefox shortcuts" actually be external .url files with the address of my homepage. That way, unless I specifically choose to open a new window (whereby I hit the new window button on my toolbar) I will always get a new tab in the current window. 

Therefore, this is why, thinking ahead, I don't want this new tab heuristic to force my external .url requests to EVER open next to the current tab.
> thinking ahead, I don't want this new tab heuristic to
> force my external .url requests to EVER open next to the current tab.

...unless it is to the far right.
(In reply to comment #29)
> Therefore, this is why, thinking ahead, I don't want this new tab heuristic to
> force my external .url requests to EVER open next to the current tab.

That won't happen with this patch, per comment 22 - comment 24.
Are you think about bookmarks? By this patch, new tabs from bookmarks are always opened at the next of the current tab. For example, if I have three tabs and the first one is selected, like:

[[Bugzilla Home]][Bug1][Bug2]

then, new tabs from a bookmark folder will opened like:

[[Bugzilla Home]][Favorite1][Favorite2][Favorite3][Bug1][Bug2] (browser.tabs.tabOpenLogic=2, browser.tabs.loadBookmarksInBackground=true)

or

[[Bugzilla Home]][Favorite3][Favorite2][Favorite1][Bug1][Bug2] (browser.tabs.tabOpenLogic=1, browser.tabs.loadBookmarksInBackground=true)

Both cases seem strange and hard-to-understood.

Moreover, tabs are opened by various context by Firefox's features and addons' features. Applying the feature for any tab open possibly cause many unexpected problems. And, if the feature is introduced, we (add-on authors) cannot use the first argument of the "addTab()" because we must use "about:blank" to ignore the feature introduced by this patch. In other words, switching behavior by the first argument of "addTab()" seems stupid idea.
Hmm, good point. Perhaps we should leave this up to the addTab caller by adding an optional parameter?
BTW, an addon "Tab History", which copies session histories of the current tab to the opened tab if it is from a link, switches its behavior by the second argument of the "addTab()" method. I think this a better way than "about:blank" trick.

(In reply to comment #33)
> Hmm, good point. Perhaps we should leave this up to the addTab caller by adding
> an optional parameter?

I think so, however, "addTab()" (and "loadOneTab()") already has too many options. It is possibly the time to replace those many options to a single hash (JavaScript object lireral). Of course we must keep backward compatibility for the legacy style (multiple options)...
Blocks: FF2SM
Let's wait for a better implementation, then.
Assignee: zeniko → nobody
Status: ASSIGNED → NEW
Attachment #356414 - Flags: approval1.9.1?
Thanks, zeniko.

There is another problem. Some people throw doubt to introducing this feature to Firefox 3.1, not to Firefox 3.2 (or later). Now, Firefox 3.1 is pre-beta3, and in the step of feature-freeze. I also love the feature, however, restructuring of basic API is dangerous and possibly breaks compatibility of addons. We have to be careful and careful. If we require much time for this bug, I think it is a better way that we suspend it now and restart it after Firefox 3.1.
(In reply to comment #33)
> Hmm, good point. Perhaps we should leave this up to the addTab caller by adding an optional parameter?

how about this:

         <parameter name="aCharset"/>
         <parameter name="aPostData"/>
         <parameter name="aOwner"/>
         <parameter name="aAllowThirdPartyFixup"/>
+        <parameter name="aTabOpenLogic"/>
         <body>
           <![CDATA[

if the caller doesn't want addtab to move the tab, just call addtab with the last new parameter set to 0 


why we need to use moveTabTo in addtab we can insert the new tab into the new place with 

-            this.mTabContainer.appendChild(t);
+            // Insert the tab to the (immediate) right of the currently
+            // selected tab, or at the end depending on the value of aTabOpenLogic
+            let index;
+            if (aTabOpenLogic === undefined)
+              aTabOpenLogic = this.mTabContainer.mTabOpenLogic;
+            if (aTabOpenLogic > 0 && (!blank || aTabOpenLogic < 3)) {
+              index = this.mCurrentTab._tPos + this.mTabContainer.mOpenedTabsCount + 1;
+              if (aTabOpenLogic == 2 || aTabOpenLogic == 4) {
+                // open the next new tab one tab further to the right
+                this.mTabContainer.mOpenedTabsCount++;
+              }
+            }
+            // when index is null it acts like appendChild
+            this.mTabContainer.insertBefore(t, this.mTabs.item(index));



>+            case "browser.tabs.tabOpenLogic":
>+              let oldTabOpenLogic = this.tabbox.mTabOpenLogic;
>+              this.tabbox.mTabOpenLogic = subject.getIntPref(data);
>+              if (this.tabbox.mOpenedTabsCount > 0 &&
>+                  (oldTabOpenLogic == 2 || oldTabOpenLogic == 4)) {
>+                // make sure not to open the next new tab in an unexpected >position
>+                this.tabbox.mOpenedTabsCount = 0;
>+              }

we only need to reset this.tabbox.mOpenedTabsCount to 0 if the NEW value is not 2 or 4

+            case "browser.tabs.tabOpenLogic":
+              let tabOpenLogic = this.tabbox.mTabOpenLogic = subject.getIntPref(data);
+              if (this.tabbox.mOpenedTabsCount > 0 &&
+                  tabOpenLogic != 2 && tabOpenLogic != 4) {
+                // make sure not to open the next new tab in an unexpected position
+                this.tabbox.mOpenedTabsCount = 0;
+              }
FWIW, I agree that:

 - new tabs opened from bookmarks
 - new tabs opened from third-party applications

should follow the same heuristic as opening a "new tab" in general (that is, at the end of the tabstrip).
Attached patch better implementation (obsolete) — Splinter Review
This will produce the desired behavior, allow to fine tune more easily by adjusting individual callers and even have less impact on extension compatibility since the new behavior is opt-in.
Attachment #356414 - Attachment is obsolete: true
Attachment #356971 - Flags: review?(gavin.sharp)
I know there is an assumption in the all tabs menu code where, if a new tab is opened while the all tabs menu is opened, the new menu item for that new tab is always added to the end. That'll likely have to be fixed (and have automated tests written for it?)
(In reply to comment #40)
That fix belongs to a different bug: the All Tabs menu doesn't react to moved tabs at all (which already breaks any extension moving tabs around while the popup is visible).
Something that I just realized -- when you close a tab, it (almost) always goes to (displays) the next on the right right.

Very often, I want it to go left.
(In reply to comment #42)
> Something that I just realized -- when you close a tab, it (almost) always goes
> to (displays) the next on the right right.
> 
> Very often, I want it to go left.

Yeah, that has been discussed ad nauseum for one direction or the other on various bug reports on here over time. While I personally favor it remaining moving to the right, and I think that would best suit this current bug fix's design, if I was to make an unbiased guess I would still argue that they would make it a WONTFIX, based solely on their past decisions, but then I can't tell for sure anymore, because actually the behavior being implemented here was a WONTFIX at one point, as well.  If they were to do it, it would seem to be most appropriate to have the tabs open 1, C, B, A, and then have Firefox know to jump to A if 1 is closed, then move to B and to C, which would be impossibly confusing to most users.  If you have it as 1, A, B, C, then moving right continues to make perfect sense.
Beltzner: Bump for Beta4?
It doesn't have the blocking firefox3.1 flag, so it will probably not be fixed in that release.
It would be a big disappointment for me not to see this change at least with the 3.1 release, let me say. I think it is a much expected behaviour and helps to spread the browser (new inexpert users don't use the extensions).
This is a simple technical change - not much code, not much complexity. Regarding getting 'The Optimal' behavior - lets get it from the users themselves. The patch as it is right now is fairly good. We can land this, get user feedback (which I am sure will be quite a lot) and see if we can tweak it in time. 

I really feel this is the right behavior and it should definitely be a part of the default set-up.
[quote]> Something that I just realized -- when you close a tab, it (almost) always goes
> to (displays) the next on the right right.
> 
> Very often, I want it to go left.

Yeah, that has been discussed ad nauseum for one direction or the other on
various bug reports on here over time. While I personally favor it remaining
moving to the right, and I think that would best suit this current bug fix's
design, if I was to make an unbiased guess I would still argue that they would
make it a WONTFIX, based solely on their past decisions, but then I can't tell
for sure anymore, because actually the behavior being implemented here was a
WONTFIX at one point, as well.  If they were to do it, it would seem to be most
appropriate to have the tabs open 1, C, B, A, and then have Firefox know to
jump to A if 1 is closed, then move to B and to C, which would be impossibly
confusing to most users.  If you have it as 1, A, B, C, then moving right
continues to make perfect sense.[/quote]
Let me clarify what I see and what I want.

Very often I will open a bunch of tabs from an index page -- for example, when reading a forum. After reading all the sub-tabs that came from that index page -- the A, B, C in your example -- I want it to go back to 1 (to the left) not forward to 2 (to the right).

I did come up with a simple way to make this behavior. It's not immediately intuitive to non-CS people.

New tabs open to the <b>left</b>, not the right. Closing then just goes to the right.

Then, if I open a bunch of tabs with ctrl-click or mac-click, I can then click on the first of the sub-tabs, and get the "read subtabs, and back to the main page" behavior.

This is almost a simple stack -- new tabs get pushed to the left, and when you move to the first, you then pop them off.

Note that this won't affect "open one new tab, then switch to it, read it, close it and return" behavior, as that will still behave just like you'd expect.
No, no, no, no, no, no, NO.

We had this behavior briefly in some nightly builds during the 0.9.x timeframe (I don't remember for sure when; might've been between 0.9.6 and 0.9.7, but I'm not sure about that anymore), and it was horrible beyond the bounds of all reason.  It's horrible in IE7 as well, to the point where tabs are basically unusable there unless you change the preference.

The old default behavior is correct.  Tabbed browsing is a FIFO queue by design.  Any different behavior should be an extension.

Please WONTFIX this, fast.
Version: unspecified → Trunk
Gavin: where are we on the review for this patch?
I've lowered the priority because I think it would be a bad idea to land it on the branch at this point, since it's likely to have pretty significant add-on impact and a relatively high risk of regression (because we're changing behavior that is often relied on implicitly).
as part of the latest design challenge marcio put together some analysis that shows just how bad the current behavior is in a very common use case of reading gmail or any web based mail app.

http://www.taboca.com/labs/p/conceptseries/tab-proximity.html

It show how people can easily get to 40 or 50 tabs and how the experience is increasingly degraded as a user tries to stay on the main task of trying to get though the inbox but wants to click on links to articles in mail and new tabs are appended.

There was a call for use cases earlier in the bug by beltzner, but I don't see any showing up.

https://bugzilla.mozilla.org/show_bug.cgi?id=262459#c13 says behavior is 'unpredicable' but its unclear to me if that means in our code, or in some kind of use case behavior.

I also think marcio's proposal is a bit different that the wontfix bug 262458 behavior of 

  A123 B12 C

marcio's proposal is more like

   A321 B21 C

with a constant insertion of the new tab to the immediate right when the tab is launched from content in the current tab.

is that what the current patch does?   could it do something like that?
Use case:

Open a bugzilla query like "bugs filed in last 24h), open a crap load of bugs to try and triage them, open first bugs attachement and the attachment is loaded after the last tab so scroll all the way down the tab strip to find attachment and then drag it all the way back to the begining so you can flip between the bug report and attachment easily.
re: comment 54
yeah, I think that is another use case of why inserting to the immediate right of the current tab seem like it is the right thing to be doing if I read it right

comment 49 , comment 43 , bug 262459 and many other parts of the discussion say there are bad side effects to changing the behavior, but I don't see any use cases that help to understand exactly what the side effects are and what the impact of the side effects might be.   It would be good to try and build those cases with some mock ups or video demonstrations, or just some comments in a bug or blog somewhere.
I think the gmail case in commment 53 and the bugzilla case in comment 54 are reflective of the pattern seen in all modern web apps.

The web app wants to be persistent, so it will do a couple of things:

-avoid the user being able to click on a link presented by the app, and have the app go away.

-make every effort to create links or rewrite user generated content to launch new windows or tab when clicking a user clicks on a link.

When we send the user to a new tab that is spacial very distant from the web app tab, we make the browser a lot harder to use.

In fact we could rename this bug "Firefox tabs don't work well with modern web apps" if it only was going the be reflective of trying to solve the problems in the last few comments.
Flags: wanted-firefox3.6?
Flags: blocking-firefox3.6?
any progress on this?

Also, I just installed 
 Tabs Open Relative 0.4
by John Mellor (Jomel)
https://addons.mozilla.org/en-US/firefox/addon/1956
442,686 downloads 199 reviews and 5 star rating.

and I think my life just got a lot better....  (so much for my life ;-)

Its doing what I want and I haven't noticed any negative side effects.  I'll keep an eye out.  

might be worth a look to see what John is doing and it can be integrated for 3.6.
>I did come up with a simple way to make this behavior. It's not immediately
intuitive to non-CS people.

>New tabs open to the <b>left</b>, not the right. Closing then just goes to the
right.

Yes, please, that was what I was asking for.

> Tabbed browsing is a FIFO queue by design.  Any different behavior should be an extension.

No. Tabbed browsing is a work-around for **** windowing systems that make creation of new windows expensive, and that make it difficult to work with many, many windows on-screen.

Don't forget that microsoft for a while tried to force its apps to be single-window by default; even MS realizes that it's windowing behavior isn't great.

What does tabbed browsing really need to be "a good thing"? Answer: tabs, and the pages that they represent, need to be independent of the window. The tab is the controller; the page is the data/model; the window currently displayed in is the view.

Instead, the current system doesn't let you move tabs freely to new windows, to existing windows, etc. (It generally reloads the url, which fails miserably for browser-state dependent web-apps.)

The current system of tabs is a workaround for the difficulty of working with lots of windows. To that point, the goal of tabs is: What?

The goal of tabs is: What?

Has anyone on the firefox dev team really considered that?

For me, the goal of tabs is to keep related pages in the same window.
Very often I will reach the point of being on an index page that isn't really related to the old stuff, but is the start of new stuff. For that, I want to either "Move tab to new window" (doesn't exist), or open a bunch of tabs in the same new window (I can open link in new window, but that only helps get the first one out).

But as long as related pages are being kept in the same window, there is another concern: New tabs should have a duplicate of the browser state, just like a "fork()" call. This would permit javascript links to be opened in a new tab.

If you are going to say that "Tabs are X by design", first show me the design specs. Tabs don't do anything useful by design.

Tabs work around limitations of the windowing system, only.

===

Under normal circumstances, what do you want to happen when you close a tab? You want to go back to where you were, probably.

What happens if you are on a forum index page? You might go down by
1. ctrl/apple-Click on interesting thread "Show new posts" link.
2. Hit tab 5 times.
3. Ctrl-enter (or apple-enter) to open another link
4. Tab 5 times
5. Repeat multiple tabs, ctrl/cmd click, until you've opened all of the threads that look interesting.
6. Now go and read them. When you are done reading, return to the index page.

Sound reasonable? What order do you want to read them? Probably the same order you opened them. That's FIFO.

What happens if thread #3 has some interesting links? You probably want to open them as you are reading thread #3, and then go and read them. When you are done, you may want to come back to thread #3, and review it in light of these comments, or you may close thread #3 and just read these articles. When done, go to thread #4. Or, you may say "These are interesting, but not relevant to what I'm doing now, they will be my next reading topic.

But if you read them, in what order? Again, probably FIFO.

** But it is localized FIFO, not global FIFO **.

With those as the typical use cases, what happens? Well, if closing a tab moves you to the right, then when you are done with the sub-tab-group, and want to come back to the index/main thread/whereever, then you want the tab to the right of the last one to be the page you were just on.

So that means "Open to the left".

If my tab bar is A, then it will become 1 A, 1 2 A, 1 2 3 A.
I then switch to tab 1, and read it. Done, close it. 2 3 A.
Read 2. Ohh, some interesting stuff. a 2 A. a b 2 A. a b c 2 A. a b c d 2 A.

The key command(s) needed?

As long as the goal is localized FIFO reading, you'll either close this tab, or move to the left-most tab.

If you've opened a group of tabs that, while interesting, isn't relevant, you want "split all tabs off to the left into a single new window".

Those behavior changes: Always open tabs to the immediate left, quickly move to the first tab, and split tabs left of me into a new window are the 80/20 solution to making tabs useful for something beyond just working around windowing system limits.
BTW, I would also would like the new parity behavior as default, I also agree this should move off the add-on only to FF parity.  

This bug should be Parity first, options later. 

So if this is already desired parity and many people have consensus we want to change the default behavior, and if its in the patch which wasn't landed on the previous branch as been sitting here, any chance this can bake on the trunk now for 3.7?  

If not, I think even a try-build based on 3.5.2 branch users can checkout and provide forum or survey feedback results on mozillazine or something, which might just push this from what if to actually help push this to reality.

Can we get something to play with?

It appears the consensus is here to be open links from the current tab to the immediate right of current tab, and ability to insert/add a new tab to right or end [via option in tab features].  

Since there is so much discussion going on here, anything and everything else should get ironed out later as separate discussions like Left or Right, New or Related Tab behavior Options in the Options Panel.

And even if you gave the user more option panel tab preferences, there should be a default option that is parity with other browsers and older FF behavior.
> It appears the consensus is here to be open links from the current tab to the
immediate right of current tab, and ability to insert/add a new tab to right or
end [via option in tab features].  

It does? It doesn't seem like that to me.

A review shows what I see as consensus is this:
1. New tabs opened while browsing a page should be near that page (currently at the end unless you use an addon)
2. New tabs opened because the web site you are on opens a page in a new window (and your settings force that to new tabs) also are near you.
3. New tabs opened because an external app opens a web page, if they open in the current window instead of a new window, should NOT be "near".

Incidentally, is there an option set to say "Frames I open while browsing go into a tab in the current window; frames opened because another app on my system opened a URL go into a new window"? (Lets call them something other than "Windows", ok? I believe this is "target=_new" in the web page. We can't call them pages, because since the first NeXT browser, most pages open in the SAME tab/window.)

FF 3.0 only has "New pages should be opened in" and gives the choice of window or tab.
Mike, that's fair enough (there was a lot to digest in this bug).  

Basically we out to just decide on a course of action and move forward.  Having said that, browser parity still should be number #1 change regardless of little issues for the 1st round of changes since the install base is big and you do have good points btw and any tweaks that make sense do have their place.

Not everyone will ever like the changes that are made.  Since breaking add-ons are a by product of progress, any branch already in stone shouldn't be broken per-say, which is what the trunk and new milestones are for.  

The browser install base is much larger than add-on usage and should not be the number one reason this bit-rots all together as Add-on's will get updated.

I would think hardcore FF users will get used to the parity behavior, and habit users will struggle with anything new.  The rest can get a new compatible Add-on if they still don't like it.  But the basic option if any should be turn on new parity tab behavior by default or use old behavior (like how that FF is not your default browser business and you could even let users make this choice on after completion of the install process but on browser startup with a msgbox or mention on the what's new release notes feature page and how to change the setting).

We should set a target milestone.
there is one more case I've seen that might need to be addressed.

* a new tab that gets created by an firefox extension/app window.

I'm loving the behavior I get with  "Tabs Open Relative 0.4" which does basically what is spec'ed in comment 60, but with one exception.  When I click on a link in a chatzilla window it opens a new tab next to the last tab window that had browser focus.  The problem is compounded when the new tab that chatzilla opens doesn't get focus, so it can be a mess poking around in windows and tab to go find the tab opened by chatzilla.   Many users may not see this so it might not be a widespread problem worthdealing with.   Anyone know of other addons that open new tabs/windows?
The "Tabs Open Relative" extension also changes the New Tab button behaviour.  Instead of opening immediately beside the New Tab button, new tabs are opened next to the last tab that had focus.
(In reply to comment #63)
Opening bookmarks in a new tab also opens next to the current tab with that extension.  I would expect the new tabs in these two cases to open at the right end of tab strip as is done now.  Opening to the right of the current tab should only be for links opened from the current tab IMO.
I have created a patch that adds a new preference to the tabs preferences
dialog enabling the user to specify if they want new related tabs to open next to the current tab rather than at the end of the tabs list.

As per the discussion in this bug and considering parity behaviour, external links and bookmarks open at the end of the tab list. To achieve this only tabs that have a referrer URI set open next to the originating tab.

Any dragging or closing of tabs results in the last remembered tab position to be reset.

For ease of testing, the patch enables this new behaviour by default, but
should perhaps be introduced as an option without changing default behaviour.
Assignee: nobody → jmorkel
Attachment #397628 - Flags: review?(dao)
How does this differ from Simon's patch in behaviour?
Re: 67, The patch behaviour is similar, but the implementation is different. This also includes a preference to switch on/off the new behaviour in the preferences dialog.
(In reply to comment #66)
> should perhaps be introduced as an option without changing default behaviour.

If there are options where to open the new tab, left of the current, right of the current, at the end of the tabs, at the end of the last visible tab or whatever else but these options would be very useful.
(In reply to comment #69)
> 
> If there are options where to open the new tab, left of the current, right of
> the current, at the end of the tabs, at the end of the last visible tab or
> whatever else but these options would be very useful.

Looks like this has been discussed in bug 262459.
Comment on attachment 397628 [details] [diff] [review]
Altering new tab behaviour as per bug discussion

>+pref("browser.tabs.insertAfterCurrent", true);

Can you add "related" to the pref name?

>+      <field name="mLastReferrerTab">
>+        null
>+      </field>
>+      <field name="mLastReferredTabPosition">
>+        null
>+      </field>    

I'm opposed to the mFoo naming scheme in JS, would prefer _foo.

>+	      try {

What exactly are you catching here?

>+                  this.moveTabTo(t, newTabPos);

moveTabTo will dispatch a TabMove event, which we shouldn't do before dispatching TabOpen. So I think all your code needs to be moved to the end of that method.

There are some tabs in your code, please make sure to use spaces only.
Attachment #397628 - Flags: review?(dao) → review-
Attachment #356971 - Flags: review?(dao)
Comment on attachment 356971 [details] [diff] [review]
better implementation

While you're at it...
I prefer John's approach for its simplicity. Any reason against using the referrer to identify related tabs?
(In reply to comment #73)
(1) Tabs being loaded through nsBrowserAccess don't get a referrer until after they're created (cf. the second change of my patch). (2) Another difference are tabs opened from the address bar, which we might also want to open next to the current one (in case you're just editing the end of the URL). (3) And I'm sure there'd be other use cases for extensions, which might or might not want to open the tab in a specific location independently of whether they pass in a referrer or not...
(In reply to comment #74)
> (1) Tabs being loaded through nsBrowserAccess don't get a referrer until after
> they're created

This looks fixable to me.

> (2) Another difference are
> tabs opened from the address bar, which we might also want to open next to the
> current one (in case you're just editing the end of the URL).

A pretty rare use case, I think. And if you're not just editing the end of the address, you're potentially getting false positives.

Extensions may use moveTabTo if they want to.
(In reply to comment #75)
> A pretty rare use case, I think. And if you're not just editing the end of the
> address, you're potentially getting false positives.

Even if you're not editing the end, I'd argue that you'd still want that tab next to the current one, as otherwise you'd just have Ctrl+T'd to get a new one, wouldn't you?

> Extensions may use moveTabTo if they want to.

That's not equivalent however when you're opening tabs in a group (i.e. if from tab A you open 1 and 2, then an extension wants to add 3 and you continue to open 4 and 5, moveTabTo alone won't get you the expected A12345B...).
(In reply to comment #76)
> Even if you're not editing the end, I'd argue that you'd still want that tab
> next to the current one, as otherwise you'd just have Ctrl+T'd to get a new
> one, wouldn't you?

I would, but I always use Ctrl+T anyway. I assume others use Alt+Enter because it saves a key stroke.

> > Extensions may use moveTabTo if they want to.
> 
> That's not equivalent however when you're opening tabs in a group (i.e. if from
> tab A you open 1 and 2, then an extension wants to add 3 and you continue to
> open 4 and 5, moveTabTo alone won't get you the expected A12345B...).

You'd get A31245B if I read the patch correctly. Since the user opened 1, 2, 4 and 5 in a row while 3 may be not directly related to that group, I'm not sure that 12345 would be expected.
(In reply to comment #77)
> You'd get A31245B if I read the patch correctly.

Which I'd expect even less if the tab were not related.

I was implying that the extension deemed the tab related, though (as that's where the two proposed API changes differ). If you depended on the referrer, a drag&go type extension couldn't open links (with referrer) and searches (without a referrer) in a consistent order, even if it wanted to (unless hacking tabbrowser innards).
Depends on: 514310
Comment on attachment 356971 [details] [diff] [review]
better implementation

this also dispatches TabOpen and TabMove in the wrong order
Attachment #356971 - Flags: review?(dao) → review-
(In reply to comment #78)
> (In reply to comment #77)
> > You'd get A31245B if I read the patch correctly.
> 
> Which I'd expect even less if the tab were not related.

3 is obviously related to A, otherwise the extension wouldn't move it. I was saying that there might be a secondary relation between 1, 2, 4 and 5 where 3 would break the chain, but that's only a wild guess, as I can't think of many extension use cases anyway.

But if we really care about referrer-less extension uses, a moveTabAfterSelected method could do the work.
(In reply to comment #79)
> this also dispatches TabOpen and TabMove in the wrong order

That's easily fixed by moving one code block. Anything else, before I consider producing a new patch?

(In reply to comment #80)
> But if we really care about referrer-less extension uses, a
> moveTabAfterSelected method could do the work.

Out of curiosity: How would that be simpler (to implement/use) than adding another parameter to loadOneTab and addTab?
>Can you add "related" to the pref name?
Done

>I'm opposed to the mFoo naming scheme in JS, would prefer _foo.
Fixed.

>What exactly are you catching here?
Whoops, meant to remove that. Fixed.

>moveTabTo will dispatch a TabMove event, which we shouldn't do before dispatching TabOpen. So I think all your code needs to be moved to the end of
that method.
Done

>There are some tabs in your code, please make sure to use spaces only.
Fixed.
Attachment #397628 - Attachment is obsolete: true
Attachment #398342 - Flags: review?(dao)
Comment on attachment 398342 [details] [diff] [review]
Revised patch altering tab behaviour with added preference

>+      <field name="_LastReferrerTab">
>+        null
>+      </field>
>+      <field name="_LastRelatedTabPosition">
>+        null
>+      </field>      

These should start with a small "l". Also, there are trailing spaces on the last line.

>+            if (!blank) {
>+              const pref = "browser.tabs.insertRelatedAfterCurrent";
>+              var moveAfterCurrent = this.mPrefs.getBoolPref(pref);
>+
>+              // aReferrerURI is null or undefined if the tab is opened from
>+              // an external application or bookmark, i.e. somewhere other
>+              // than the current tab.
>+              if (moveAfterCurrent && aReferrerURI) {

The blank check isn't needed (in fact, I think it's wrong), and the pref can be read after checking the referrer:

if (aReferrerURI &&
    this.mPrefs.getBoolPref(...))

>+                var newTabPos;
>+                if (this.mCurrentTab == this._LastReferrerTab) {
>+                  newTabPos = ++this._LastRelatedTabPosition;
>+                } else {
>+                  newTabPos = this.mTabContainer.selectedIndex + 1;
>+                  this._LastReferrerTab = this.mCurrentTab;
>+                }
>+                this._LastRelatedTabPosition = newTabPos;                
>+                this.moveTabTo(t, newTabPos);
>+              }
>+            }

How about just keeping track of the last related tab?
Something like:

let newTabPos = (this._lastRelatedTab ||
                 this.selectedTab)._tPos + 1;
this.moveTabTo(t, newTabPos);
this._lastRelatedTab = t;

I guess _lastRelatedTab should be reset in _endRemoveTab, moveTabTo and updateCurrentBrowser.

>             if (dropEffect != "link") { // copy or move
>               draggedTab = dt.mozGetDataAt(TAB_DROP_TYPE, 0);
>               // not our drop then
>               if (!draggedTab)
>                 return;
>             }
> 
>             this.mTabDropIndicatorBar.collapsed = true;
>+            this._LastReferrerTab = null;
>             aEvent.stopPropagation();

This is the wrong place for this, I think. As I said above, that should probably be done in moveTabTo.
Attachment #398342 - Flags: review?(dao) → review-
(In reply to comment #81)
> That's easily fixed by moving one code block. Anything else, before I consider
> producing a new patch?

As a whole, it seems unnecessarily complex to me.

> Out of curiosity: How would that be simpler (to implement/use) than adding
> another parameter to loadOneTab and addTab?

to implement: It's a single code block at the end of addTab in John's patch. Making that a separate method would be most trivial.

to use: These methods already suffer from too many arguments. You can hardly use them with more arguments than the URI without looking at their signature. For something that I wouldn't expect most callers to deal with and that can be moved to a separate method, I think it makes sense to avoid the extra argument.
>I guess _lastRelatedTab should be reset in _endRemoveTab, moveTabTo and updateCurrentBrowser.

I've decided to not reset it in updateCurrentBrowser and rather keep _lastReferredTab to check if the referrer tab for the related tab we're opening is the same one that opened the last related tab. This way a user can change tabs and so long as the order doesn't change by dragging (MoveTabTo) or closing tabs (_endRemoveTab), the tab will open next to the last related tab.

I hope I'm being clear enough. Would this behaviour be desirable?

Fixed all the other issues.
Attachment #398342 - Attachment is obsolete: true
Attachment #398362 - Flags: review?(dao)
(In reply to comment #85)
> I've decided to not reset it in updateCurrentBrowser and rather keep
> _lastReferredTab to check if the referrer tab for the related tab we're opening
> is the same one that opened the last related tab. This way a user can change
> tabs and so long as the order doesn't change by dragging (MoveTabTo) or closing
> tabs (_endRemoveTab), the tab will open next to the last related tab.

Why is it important or even desirable to keep adding tabs after the last related one if the user selected tabs in the meantime? Starting with a new group of related tabs seems more predictable to me.
>Johnson: what I was meaning is if you have tabs A B C and open 3 new tabs from A
>Johnson: so you now have A 1 2 3 B C
>dao:     right
>Johnson: and you change to tabs 1 2 and 3 to look at them then change back to A
>dao:     or B, or C, and then go back to A
>Johnson: then open two more tabs
>Johnson: with my suggestion: you'll have A 1 2 3 4 5 B C
>dao:     I suggest you get A 4 5 1 2 3 B C
>Johnson: okay, at least you understand me then
>Johnson: you reckon that's the best behaviour?
>Johnson: I'm suggesting that so long as the order doesn't change (through dragging or tabs closing) then the new tabs can be appended to the list of related tabs so to speak
>dao:     it's more predictable, I think. if the user interacted with various tabs, he might not even remember the relation between A and 3 anymore
>Johnson: I suppose
>Johnson: okay, I'll change it quickly and resubmit
>dao:     mind if I post our conversation to the bug so that everyone is up to date?
>Johnson: sure
Attached patch Final attempt for checkin? (obsolete) — Splinter Review
This patch implements the behaviour suggested in comment #87 as well as the various other changes suggested by Dão.
Attachment #398362 - Attachment is obsolete: true
Attachment #398368 - Flags: review?(dao)
Attachment #398362 - Flags: review?(dao)
Attachment #356971 - Flags: review?(gavin.sharp)
Comment on attachment 398368 [details] [diff] [review]
Final attempt for checkin?

>+            // the current tab has changed so reset last related tab tracker
>+            this._lastRelatedTab = null;

This doesn't seem like a useful comment, you might as well remove it. Same in _endRemoveTab and moveTabTo...

>+              let newTabPos = (this._lastRelatedTab ||
>+                              this.selectedTab)._tPos + 1;

this.selectedTab should be indented one space further.
Attachment #398368 - Flags: review?(dao) → review+
(In reply to comment #84)
> Making that a separate method would be most trivial.

Then please do so. I'll leave deciding whether this is the right API change to the sr. My concerns are comment #74, 76 and 78.

> to use: These methods already suffer from too many arguments.

That doesn't mean that we have to move code to other methods, though, as the whole addTab API could also be reworked to just take a URL and a hash of optional arguments: gBrowser.addTab(aURL, { openTabRelative: true }) (see comment #33 and following).

Also: Please don't forget to get ui-r on the exposed option, which IMO should just be dropped (not adding a visible option has already got beltzner's ui-r+). And the tests from attachment 356971 [details] [diff] [review] should still be mostly usable for the different approach as well...
(In reply to comment #90)
> (In reply to comment #84)
> > Making that a separate method would be most trivial.
> 
> Then please do so. I'll leave deciding whether this is the right API change to
> the sr. My concerns are comment #74, 76 and 78.

This could happen in a separate bug. As is, the patch probably doesn't need sr, as there's no API change.

> > to use: These methods already suffer from too many arguments.
> 
> That doesn't mean that we have to move code to other methods, though, as the
> whole addTab API could also be reworked to just take a URL and a hash of
> optional arguments: gBrowser.addTab(aURL, { openTabRelative: true }) (see
> comment #33 and following).

It could, but that's not quite within this bug's scope.
(In reply to comment #91)
> as there's no API change.

The aReferrerURI parameter does now have further side-effects which could be relevant for extensions (cf. comment #78). Could be mistaken for an API change...

> It could, but that's not quite within this bug's scope.

Indeed not. Depending on such a bug, one further argument could have been added to addTab, though. Anyhow, do you expect to fix Alt+Enter not respecting the new pref in a different bug as well?
(In reply to comment #92)
> The aReferrerURI parameter does now have further side-effects which could be
> relevant for extensions (cf. comment #78). Could be mistaken for an API
> change...

The TabMove event is dispatched, so extensions can be expected to deal with it. That's also independent of whether we look at aReferrerURI or a separate argument.

> Indeed not. Depending on such a bug, one further argument could have been added
> to addTab, though.

The current patch also doesn't rule this out.

> Anyhow, do you expect to fix Alt+Enter not respecting the
> new pref in a different bug as well?

I see Alt+Enter as a shortcut for Ctrl+T and Enter or as an alternative to middle-clicking autocomplete results, and I wouldn't consider the way the patch handles this a bug.
Please also add a browser chrome test which covers this new feature. It should be entirely possible.
Flags: in-testsuite?
Attached patch Latest patch with tests added (obsolete) — Splinter Review
I've added mochitest code adapted from the original patch (thanks Simon).

This patch should be good for checkin if Dao thinks the tests are good.
Attachment #398368 - Attachment is obsolete: true
Attachment #398458 - Flags: review?(dao)
Attachment #398458 - Flags: ui-review?(beltzner)
We should leave out the UI for this, don't see why it needs UI, and I haven't
seen a strong case that contradicts the quote in comment 0.
I for one would want to turn it off, as I have four static tabs at the beginning of my tabstrip, across sessions. I don't want tabs opened from there to change this setup.

Of course, I could personally live with the hidden pref.
FWIW: I think the ability to change the setting in a hidden pref would be minimally desired and while I tend to agree with comment 0 in general: I want to throw this out there.. just to say we at least brought it up for those non-techies.

Being able to change it via UI easily like 'open new windows in tab', and 'switch immediately to a new tab' seems to fit with adding 'open relative tab near current tab'.  If these two first items were not already in there, I'd for-go even trying to give a reason.  

If comment 0 is important across the board, why we still have tab preferences UI anymore, even though not many people probably change the settings, doesn't seem to fit.

UI or not UI probably boils down to this: 

UI is not so the for technical who can live with hidden prefs, but for the non technical user base - *people we wouldn't want to alienate and would ask us why the habit behavior is different and how can they change it.  

[*ie someone like mom or who is a linear thinker and cannot think outside the box long enough and have patience enough to figure out how to live with the new behavior and who can help them if all they have is a hidden pref.  People tend to gravitate to software they like to use and works for them, but don't care to spend hours of their life trying to customize hidden software prefs.  If for some reason half the internet users don't like the new parity settings.. well i can see the gripes from the marketplace coming without a UI setting.]

If you compare hidden software prefs to say don't change it unless you know what your doing or you'll break your software, makes sense, but in this case your saying we're not going to give the user an easy way of changing the behavior to their liking if we leave out the UI.
I agree with comment #98, the exposed preference to toggle this behavior in the options menu would compliment the existing preferences located here to alter the default tab behavior and in turn allowing the users more customization of their tabs.  With such a major change to the behavior of tabs, an easy way to revert back to the "old way" would be the best thing to do here instead of a hidden preference.
Comment on attachment 398458 [details] [diff] [review]
Latest patch with tests added

Everything about this is great, except for the UI visible preference.

I don't mind there being a hidden pref, but the last thing we need is more options, as per comment 0.

This gets my uir+ with the checkbox removed.
Attachment #398458 - Flags: ui-review?(beltzner) → ui-review-
UI preference has been removed as per comment #0. Mochitests are included.
Attachment #398458 - Attachment is obsolete: true
Attachment #398605 - Flags: ui-review?(beltzner)
Attachment #398605 - Flags: review?(dao)
Attachment #398458 - Flags: review?(dao)
(In reply to comment #101)
> Created an attachment (id=398605) [details]
> UI preference removed. Tests included.

You missed to add browser_relatedTabs.js to your patch?
Forgot to hg add browser_relatedTabs.js. All should be there now. Thanks for spotting Henrik.
Attachment #398605 - Attachment is obsolete: true
Attachment #398614 - Flags: ui-review?(beltzner)
Attachment #398614 - Flags: review?(dao)
Attachment #398605 - Flags: ui-review?(beltzner)
Attachment #398605 - Flags: review?(dao)
Comment on attachment 398614 [details] [diff] [review]
browser_relatedTabs.js included this time

>+    addTab("http://localhost:8888/#2",gBrowser.mCurrentBrowser.currentURI);

Use gBrowser.currentURI instead. (And otherwise gBrowser.selectedBrowser instead of mCurrentBrowser.)

Looks good, thanks!
Attachment #398614 - Flags: review?(dao) → review+
I just noticed that when I tried the tryserver builds from the patch I ui-reviewed, it broke the back button. Please make sure that's fixed in this latest version of the patch.
I don't think that patch could have broken the back button.
@beltzner: this patch doesn't affect back button behaviour in any way.

So long as this gets ui-r+ it should be good for checkin.
Attachment #356971 - Attachment is obsolete: true
Attachment #398614 - Attachment is obsolete: true
Attachment #398639 - Flags: ui-review?(beltzner)
Attachment #398614 - Flags: ui-review?(beltzner)
So when it lands, is it on by default or off by default?
The new behavior is enabled by default.
Attachment #398639 - Flags: ui-review?(beltzner) → ui-review+
Comment on attachment 398639 [details] [diff] [review]
Final patch for checkin

uir=beltzner
Keywords: checkin-needed
Ok, since this a long work in progress, this would make for a nice "Labor" Day checkin.  Since we'll all be laboring differently after this. ;-)
http://hg.mozilla.org/mozilla-central/rev/91096486c92c
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
BTW: It might have been better to keep the tests for when the newly added pref
is set to false - otherwise you'll just have introduced a poorly tested code
path where regressions (as rare as they might be) won't be detected until a
user stumbles over them.
Note: correct me if I'm wrong here, but this patch also doesn't override just clicking on links with target="new" or target="_blank" which opens those links in new tabs all the way to the right with 'open new windows in new tab' = 'true', and not relative.  Nor does it invoke any closing back to the parent behavior as such like when 'switch to tabs immediately' = 'true'.
Depends on: 514794
Depends on: 514796
Depends on: 514799
cuz84d, I'm not sure what your asking in the first part but your second part of your comment is a separate bug 514796.  Perhaps file a new bug for what you are asking.

Dao, very sorry if this has been answered (I didn't see it) but how are we informing accessibility users that the tab is not opening at the end of the tab bar anymore.  I'm not sure how all of the tools they use work but I can assume their will be some confusion and have to tab through (or switch though tabs some other way) a lot more now to just find the newly opened tab.
Depends on: 514836
I've opened bug 514836 for what cuz84d described in comment 114 (and also for window.open).
Summary: Change 'Default' Tab Opening Behavior/Position for Firefox → tabs opened from links should appear next to the current tab (instead of at the end of the tabstrip)
Hello, a change-disliker here.

This change landed on trunk on Sep 4 (changeset 91096486c92c). I didn't realised what was different until now. *But* I was annoyed at the tabs, just I didn't detect what it was.
Once I saw it was consistently adding the tab after the active one, and it wasn't a buggy "I feel like opening it one before the last tab", it was easy to hg log | grep tab, and reach here.
More people will come here wondering about this preference.

So for the record, the associated hidden preference is
 browser.tabs.insertRelatedAfterCurrent
We probably have to update some litmus tests to reflect the new behavior. I'll check that in the next weeks. Doa, how good does the automated test cover this new feature? Do we have to add a manual test too?

Verified fixed on trunk with builds on OS X and Windows like Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090907 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20090907051047
Status: RESOLVED → VERIFIED
Flags: in-litmus?
The test doesn't cover bug 514310 and it doesn't cover browser.tabs.insertRelatedAfterCurrent=false, as Simon noted.
Should the test cover browser.tabs.insertRelatedAfterCurrent=false?
No longer depends on: 514796
Sadly, I wouldn't block on this, but I think we should get it in to mozilla-1.9.2 before beta.
Flags: blocking-firefox3.6? → blocking-firefox3.6-
Keywords: checkin-needed
(In reply to comment #130)
> Should the test cover browser.tabs.insertRelatedAfterCurrent=false?

If it is not uncommon to run tests with hidden prefs I would like to see such a test. Can a test for bug 514310 easily added or should it be a separate test. Dao, what do you think?
That would be up to the person writing the test. Both ways are reasonable.
Blocks: 515635
Flags: wanted-firefox3.6? → wanted-firefox3.6+
(In reply to comment #105)
> I just noticed that when I tried the tryserver builds from the patch I
> ui-reviewed, it broke the back button. Please make sure that's fixed in this
> latest version of the patch.

Mike, what did you see here.  I just ran into the back button opening new tabs instead of loading the page in the same tab.

(In reply to comment #115)
> Dao, very sorry if this has been answered (I didn't see it) but how are we
> informing accessibility users that the tab is not opening at the end of the tab
> bar anymore.  I'm not sure how all of the tools they use work but I can assume
> their will be some confusion and have to tab through (or switch though tabs
> some other way) a lot more now to just find the newly opened tab.
or anybody??
Marco:

(In reply to comment #115)
> Dao, very sorry if this has been answered (I didn't see it) but how are we
> informing accessibility users that the tab is not opening at the end of the tab
> bar anymore.  I'm not sure how all of the tools they use work but I can assume
> their will be some confusion and have to tab through (or switch though tabs
> some other way) a lot more now to just find the newly opened tab.
> tabs opened from links should appear next to the current tab

Why should? Almost all browsers by default open new tabs rightmost and you are changing a setting which most users aren't get used to and I'm sure will be opposed.

At least there HAS to be a UI setting for that - not just an about:config.

A strange counter productive decision I would say. The best way to go would have been to enable this new UI behaviour only for NEW users with empty profiles. I really hate when a default behaviour of my browser is changed behind the scenes without my content or explanations.
I agree, that a GUI option would really make sense for this setting, as it might annoy lots of people.

When I tried that behavior once with Chrome, I really liked it. But now testing it in "real life scenarios" it becomes really annoying to me. The reason it is annoying to me is the same as explained by another user before: I have proabably a fixed set of 10 tabs at the beginning of the tab strip from which i open lots of other tabs. With the new behavior it's much harder to keep this arrangement intact.
OTOH, I prefer the open at the end of the list - I usually keep 4-5 tabs open as well, but if I have, say, CNN, open as the third tab, and I open a ton of article links in new tabs, my 4th and 5th tabs move to the end.  This can get to be a problem if I then start closing tabs at the end indiscriminately and accidentally close on of my regular tabs without realizing it - until I have worked it out of the close tabs list queue.

For me I prefer the open at the end of the list - however, I also prefer it to be ***user-selectable*** via the UI as opposed to having to use a third party extension or else making modification in about:config.
So we have requests for user-selectable from:
1. Always on the far right.
2. Always the "immediate next right" (gives A B 1 2 3 C for opening tabs from page B)
3. Far right for external opens, immediate next right for internal opens
4. Always the immediate left (gives A 1 2 3 B C for opening tabs from page B)
5. Far right for external opens, otherwise the immediate left.

1, 2, and 3 require tracking the "Where to return to" tab when a tab is closed; 4 and 5 do not.

Anyone else see any "desired tab behaviors"? (Other than forcing external opens to a "same new" window -- if an external app opens 4 links, they all go into the same window, but different from any existing window.)
That seems to be it - however, there seems to be one or two additional options that would require some sort of tracking as well, as seen here http://tmp.garyr.net/help/#Events_-_Tab_Opening

It seems you've already taken not account the changing of the order of the tabs so that the oldest opened tab is directly next to the parent tab, but in (4) it seems to be reversed....is that relevant?

The other would be for duplicated tabs only.  I *think* that would require tracking, but not sure.

The same new window idea is intriguing...but I make use of single window mode anyway, so....

Of course, this is only my opinion, others should chime in as well....
> It seems you've already taken not account the changing of the order of the tabs
so that the oldest opened tab is directly next to the parent tab, but in (4) it
seems to be reversed....is that relevant?

I think what you're saying is that I'm not taking into account changing the order of tabs so that the oldest tab is next to the parent tab. In case #2, that's what you have. That's "tabs open relative" today -- opening 1,2,3 from B gives A B 1 2 3 C. 1 is the oldest tab opened from B, and it's next to B.

Case #4 is what I've argued should be the behavior. You can then go to 1, read (and close) 1, 2, 3, and wind up back at B. Otherwise, you read and close 1, 2, 3, and wind up on C, not back at B.

What you seem to be asking for is "Immediate right" -- A B 3 2 1 C. I'm trying to understand when you would want that, but we can add that to the want list.

6. Always the immediate right (gives A B 3 2 1 C for opening tabs from page B)
7. Far right for external opens, immediate right for internal opens
My apologies - I spell checked, but apparently it change my misspelled 'into' to 'not' in the following:  "It seems you've already taken ***not*** account the changing" - making that much harder to read than I intended.  What I meant was that if you're using immediate *left* and you open tabs, shouldn't the order proceed A 3 2 1 BC for tabs opened from B if you were to follow the same procedure from 2...

Me personally, I don't use the open new tabs relative to current tab option as I tend to open either 3-4 tabs or 20+ at any given time, so tab relativity is irrelevant to me.  I was just suggesting additional options as found in TMP, if it is feasible to include them in the main app code.

That being said, yes, options 6 would be nice to have, and 7 would be *my* ultimate dream.
>  What I meant was that if you're using immediate *left* and you open tabs, shouldn't the order proceed A 3 2 1 BC for tabs opened from B if you were to follow the same procedure from 2...

No. A 1 2 3 B C is "immediate left". The intended usage is open some tabs in order, then go back to 1 and read them in the order you open. When you are done with that set of sub-tabs, you are back at B.

This is "Local FIFO" -- First in, first out, for the local subset.

Think "breath first" searching. Heck, think "Du" or "find" -- examining the current directory, then go to the items in the subdirectory, returning to the local directory.
This doesn't seem to work on the second build candidate for 3.6b1 on Linux, probably it didn't work in earlier builds. Reopening. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b1) Gecko/20091019 Firefox/3.6b1
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
My mistake. I misread this as opening new tabs, vs new tabs from links.
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Sorry, missed the verification on 1.9.2. Works fine with builds on all platforms like Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b2pre) Gecko/20091016 Namoroka/3.6b2pre (.NET CLR 3.5.30729) ID:20091016050535
Keywords: verified1.9.2
removing user-doc-needed
We can't find any instances in the KB that specify where the new tab is opened. The closest is <https://support.mozilla.com/en-US/kb/Tabbed+browsing#Opening_tabs>, but I don't see a need to add this info. Anyone disagree?
Keywords: user-doc-needed
FWIW, I do Chris.  The behavior changed from how it has been for the past five years (for end users) so it might be helpful for a user doc if they go searching on why the tabs are now suddenly opening "in random places" on the tabbar.
Chris, i also disagree with removing that keyword.  Unless you've talked with sheppy about that, the keyword is needed so can put it on the list to update the user doc.  Its needs a change now with 3.6+  And since he hasn't yet, please don't remove first.
Kurt,
Creating a separate article for users who not being able to find their new tab sounds reasonable. I have filed an article bug for that (bug 530247). I did a search of 3.6 users in the support forum, and found three instances of this question in the past 3 weeks. There is a bit of a policy issue, but I would rather take that discussion to the other bug.

Dale, 
Sheppy handles dev-docs, not user-docs.
Blocks: 530247
Keywords: user-doc-needed
(In reply to comment #153)
> Kurt,
> Creating a separate article for users who not being able to find their new tab
> sounds reasonable. I have filed an article bug for that (bug 530247). I did a
> search of 3.6 users in the support forum, and found three instances of this
> question in the past 3 weeks. There is a bit of a policy issue, but I would
> rather take that discussion to the other bug.
> 
> Dale, 
> Sheppy handles dev-docs, not user-docs.

Fair enough, thanks for the update.
No longer depends on: 514836
Blocks: 539594
Setting in-litmus+. All tests have been already updated a while back.
Flags: in-litmus? → in-litmus+
Blocks: 540125
No longer depends on: 542166
WONTFIX UI option to disable this new feature unacceptable.  Issue https://bugzilla.mozilla.org/show_bug.cgi?id=539986 open due to this.

If people believe this should work like IE7, then let those people use IE7 instead.  People who have trusted and used FF since the beginning have relied on the previous tab ordering functionality, and this issue has broken that.

It should defaulty work like FF has in the past, unless you give an option to set FireFox up like FF or IE as part of the wizard when first installed.

Tools/Edit->Options/Preferences->Tabs should have this option to change between IE and FF tab ordering behavior!

The about:config way of configuring FF to previous tab ordering is not available for normal users, as it states that this may break your warrante.

As such, this issue should not be fixed.
(In reply to comment #158)
> WONTFIX UI option to disable this new feature unacceptable.  Issue
> https://bugzilla.mozilla.org/show_bug.cgi?id=539986 open due to this.
> 
> If people believe this should work like IE7, then let those people use IE7
> instead.  People who have trusted and used FF since the beginning have relied
> on the previous tab ordering functionality, and this issue has broken that.
> 
> It should defaulty work like FF has in the past, unless you give an option to
> set FireFox up like FF or IE as part of the wizard when first installed.
> 
> Tools/Edit->Options/Preferences->Tabs should have this option to change between
> IE and FF tab ordering behavior!
> 
> The about:config way of configuring FF to previous tab ordering is not
> available for normal users, as it states that this may break your warrante.
> 
> As such, this issue should not be fixed.

No offense, but the 'voiding the warranty' statement is a jest.

There is no warranty with using Firefox.

For reference, see the following websites (just the first three from a Google Search).

http://www.lifespy.com/2008/firefox-news-firefox-3-aboutconfig-changes-may-void-warranty/

http://www.firefoxfacts.com/2008/05/18/firefox-3s-aboutconfig-warning/

http://www.automaticable.com/2008-03-11/void-my-firefox-warranty/

IOW - it is there as a jest, just like the Beta builds had other messages (such as there be dragons here).
The person who I am trying to help out by fixing the tab ordering happens to be 73 years old.  It may seem a jest to you, but to the average user who knows nothing about this about:config, it is not.  It is a major hassle, and even an impossible option in some circumstances and for some people.

There are many settings in about:config which should not be randomly changed.  Hence the warning.  But there is an extention which can be downloaded and installed to do the same thing.

It was hard to get this person to download the extention and convince them that this is not a virus, but I managed it, and this is the option I used.  But this is not a har, har, har issue.
Depends on: 558614
Blocks: 574385
Minor bump.

Where is the current discussion of tab behavior happening?
This now seems to be broken, at least on linux, within the last few weeks.  Is this an intentional decision or should this bug be reopened?
(In reply to Jeff Hammel [:jhammel] from comment #162)
> This now seems to be broken, at least on linux, within the last few weeks.

WFM

> Is this an intentional decision or should this bug be reopened?

Neither. If you think you found a regression, please file it.
Comments #140 and 142 discussed usage cases. There's more than one "obvious" choice for how new tabs should behave.

As much as firefox seems to be "Have it your way", this seems to be "There is only one way".

Where is the discussion of how tab behavior should be, and how it should be customizable, currently going on?

I still maintain that the current behavior, as much as it may be duplicating what IE started years ago, is as broken now as it was when IE started, and "duplicating everyone else's broken behavior" is not a good goal. Otherwise we'd still be duplicating 80 character fixed width lines.
(In reply to Dão Gottwald [:dao] from comment #163)
> (In reply to Jeff Hammel [:jhammel] from comment #162)
> > This now seems to be broken, at least on linux, within the last few weeks.
> 
> WFM

Strange. On linux nightly?

> > Is this an intentional decision or should this bug be reopened?
> 
> Neither. If you think you found a regression, please file it.

done: bug 705478
No longer blocks: FF2SM
You need to log in before you can comment on or make changes to this bug.