Last Comment Bug 440093 - Don't overwrite current tab when opening multiple bookmarks in tabs
: Don't overwrite current tab when opening multiple bookmarks in tabs
Status: VERIFIED FIXED
[qa!][testday-20110930]
: dataloss, verified-aurora
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: 3.0 Branch
: All All
: -- major with 9 votes (vote)
: Firefox 8
Assigned To: Jez Ng [:int3]
:
Mentors:
: 510094 514617 567326 581796 587132 658774 672018 (view as bug list)
Depends on: 683146 705465 395024
Blocks: 589079 645010 647298 630856 675887
  Show dependency treegraph
 
Reported: 2008-06-18 12:25 PDT by Jeff Largent
Modified: 2013-11-12 00:00 PST (History)
32 users (show)
hskupin: in‑litmus?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (992 bytes, patch)
2011-06-26 10:20 PDT, Jez Ng [:int3]
no flags Details | Diff | Splinter Review
Patch v1 [more lines of context] (1.46 KB, patch)
2011-06-26 10:31 PDT, Jez Ng [:int3]
faaborg: feedback+
Details | Diff | Splinter Review
Patch v2 (1.35 KB, patch)
2011-07-31 00:01 PDT, Jez Ng [:int3]
mak77: review+
Details | Diff | Splinter Review
Patch v2 [for checkin] (1.36 KB, patch)
2011-08-02 10:55 PDT, Jez Ng [:int3]
no flags Details | Diff | Splinter Review

Description Jeff Largent 2008-06-18 12:25:45 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052912 Firefox/3.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052912 Firefox/3.0

When opening multiple pages in tabs from bookmarks open all in tabs option.  The tab that is in focus is replaced but others are left.  I have my setting so that it should open in new tabs not current tabs.

Reproducible: Always

Steps to Reproduce:
1.have several tabs open to different sites
2.from bookmarks select open all in tabs option from a folder with multiple site bookmarked
3.
Actual Results:  
Tab that is in focus is replaced all others stay.

Expected Results:  
All should be opened in new tabs leaving existing tabs unchanged.

I have my config set to open in new tabs not replace existing tabs.
Comment 1 Arthur Tam 2008-06-18 23:30:32 PDT
I got the same problem where my OS is Windows 2000 professional and the Firefox 3.0 version was downloaded on 18 Jun 2008.
Comment 2 Alan Arndt 2008-09-26 13:04:36 PDT
I had never noticed this as an issue.  That's apparently because I use middle click on the Bookmark Folder.  That correctly opens in all new tabs, while selecting the menu option "Open All in Tabs" from a right click does replace the current tab with the first tab of the folder of bookmarks.

It has behaved this way for me with FF 3.0 and later on Windows XP.
Comment 3 Aakash Desai [:aakashd] 2009-01-16 15:11:01 PST
I see the same issue on Shiretoko 3.1b3pre on a Windows XP machine.
Comment 4 Jonadab the Unsightly One (Nathan Eady) 2009-03-28 03:44:55 PDT
This is happening to me in Iceweasel 3.0.6 on Debian stable (lenny).  Yesterday I couldn't find a tab that I knew I had open...  it was gone.  (So that's surreptitious dataloss, which IMO should give the bug a rather higher than "normal" severity.)  This morning I was paying more attention and happened to actually see what happened:  when I opened a folder of bookmarks in tabs (my morning comics), the first one replaced the contents of the current tab.

browser.tabs.loadFolderAndReplace is set to false.

Reproducability:  High.
Steps to reproduce:
1.  Open some tabs.
2.  Focus a tab.  Any tab.  Not necessarily the rightmost.
3.  For a folder on the bookmarks toolbar, select "Open all in tabs".

Expected results:
N *new* tabs should be opened, where n is the number of items in the folder.  Ideally they should be opened at the far right of the list of tabs.

Actual results:
N - 1 new tabs are opened, directly to the right of the current one, for the second and following items in the folder, but the first item in the folder is loaded in the current tab.  If the user finishes reading the first item and hits close tab instead of back, the contents of the original tab are lost.  (Okay, with Undo Closed Tab they are theoretically still recoverable, but if you don't discover what you lost until later, unclosing enough tabs to recover the lost one could be a real pain.)
Comment 5 Jonadab the Unsightly One (Nathan Eady) 2009-04-01 04:11:38 PDT
Also happens on the official Mozilla Firefox 3.0.8 build on Linux.
Comment 6 Jonadab the Unsightly One (Nathan Eady) 2009-04-29 04:11:53 PDT
I thought (still think) it's odd that not very many people are reporting this, so I suspected it might be caused by an extension or other profile issue.  So I logged in as a test user that had never used Firefox before, so as to have a clean profile, but I was disappointed.  The only difference is that in the clean profile browser.tabs.loadFolderAndReplace was true initially (can't imagine who thought that was a good idea, but I make it a policy not to complain too hard about defaults, provided they can be changed), so I had to fire up about:config and toggle that first in order to produce the bug as described.  (The behavior if you don't toggle the pref is, of course, N times worse, where N is the number of tabs.)
Comment 7 Alan Arndt 2009-04-29 08:39:17 PDT
What I find most interesting (annoying) is that it 1) Changed from the prior versions (< 3.0) and 2) It's inconsistent.  Middle click on links on a page that then open int he background behave properly, they open and do not take focus.  However, multiple tabs opened from a bookmark do take focus and assign it to the first tab of the group.  It's just wrong.
Comment 8 Jonadab the Unsightly One (Nathan Eady) 2009-05-13 03:28:43 PDT
Alan, you may want to fire up about:config and see if browser.tabs.loadBookmarksInBackground makes any difference.

In any event, I'm pretty sure that's a separate issue.
Comment 9 Jeff Largent 2009-05-13 04:53:02 PDT
I have tried with browser.tabs.loadBookmarksInBackground set both ways and it makes no difference.

Also I have noticed that when you alt click on a folder it opens all the bookmarks in that folder without replacing any of the existing tabs.
Comment 10 Alan Arndt 2009-05-13 09:17:40 PDT
Jeff is correct, it makes no difference.  I have Load Bookmarks In Background set to true.  If I middle click on an individual bookmark it loads in another window, in the background, which is correct.  If I turn the option off and click on that individual bookmark it loads in the foreground.  That all works.

The problem is that when a group of bookmarks is loaded (a whole folder), it pays no attention to the flag and it loads the first bookmark of the group in the foreground no matter what.
Comment 11 Jonadab the Unsightly One (Nathan Eady) 2009-07-01 03:36:42 PDT
Bug is unchanged in Firefox 3.5.
Comment 12 Henrik Skupin (:whimboo) 2009-08-20 03:00:09 PDT
According to the following litmus test (https://litmus.mozilla.org/show_test.cgi?id=6529) all tabs should open in new tabs when doing a Ctrl+Shift-Click on the "Open All in Tabs" menu entry. The currently open tab should not be used for the first bookmark.

Confirming bug with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3pre) Gecko/20090813 Shiretoko/3.5.3pre ID:20090813031642.
Comment 13 Henrik Skupin (:whimboo) 2009-08-20 03:04:10 PDT
Litmus test https://litmus.mozilla.org/show_test.cgi?id=8564 for the 3.6 branch also has to be updated.
Comment 14 Henrik Skupin (:whimboo) 2009-08-20 03:09:43 PDT
Dao, it would be great if you could tell us what is the expected behavior for the following cases:

* Click on "Open All in Tabs"
* Ctrl+Shift+Click on "Open All in Tabs"

Another test which needs an update too:
https://litmus.mozilla.org/show_test.cgi?id=6531
Comment 15 Henrik Skupin (:whimboo) 2009-09-01 10:15:37 PDT
This bug should be fixed by a patch on bug 395024.
Comment 16 Henrik Skupin (:whimboo) 2009-09-04 05:15:43 PDT
*** Bug 514617 has been marked as a duplicate of this bug. ***
Comment 17 Marco Bonardo [::mak] (Away 6-20 Aug) 2009-10-16 07:16:54 PDT
this bug should be fixed here.
in https://bugzilla.mozilla.org/show_bug.cgi?id=395024#c65 Faaborg said:
" In terms of trying to
open everything in the folder, I personally think these should open in addition
to the current set of tabs, since opening them individually will cause them to
open in addition to the current set of tabs, and it would be odd for the
behavior to switch once you are acting on multiple items."

I agree, we should not overwrite the currently focused tab, if user wants to have a window with only those tabs, he can open one with a shift+click on the folder.
Comment 18 Jonadab the Unsightly One (Nathan Eady) 2009-10-21 04:26:02 PDT
This issue is not going to be fixed in bug 395024.  All that's happening there is the (now ineffectual) item will be removed from prefs.js (and thus not listed in about:config unless the user creates it).
Comment 19 Henrik Skupin (:whimboo) 2009-10-21 08:34:27 PDT
Right, my comment 15 was been made before we started to work on bug 395024. So it's still valid.
Comment 20 Scott F 2010-01-19 12:34:37 PST
Something went amiss...
Behaviour described in bug initial description is present in 3.6 RC2 and is persistent. Further, previously opened tabs are "pushed right" to make way for the group of bookmarks being opened (as opposed to appending the new bookmarks to the end of the last opened tab).
Comment 21 Henrik Skupin (:whimboo) 2010-07-25 06:28:39 PDT
*** Bug 581796 has been marked as a duplicate of this bug. ***
Comment 22 Jonadab the Unsightly One (Nathan Eady) 2010-08-25 03:43:19 PDT
QA: Ignore (this comment is off-topic for this bug)
Scott F, regarding the new tabs going in the middle of the existing tab list instead of at the right, there's a new item in about:config that you have to set in 3.6 to get the correct behavior back.  I don't remember right now exactly what it's called (I'm running 2.0.0.20 here, mostly because of bug 440093), but if you search for "tabs" in about:config you should find it.  I think it has the word "after" in it, or a synonym.  It's a boolean that defaults to true; setting it to false causes all new tabs to open at the right end as they should.
Comment 23 Henrik Skupin (:whimboo) 2010-11-24 01:33:05 PST
*** Bug 457308 has been marked as a duplicate of this bug. ***
Comment 24 Henrik Skupin (:whimboo) 2011-02-06 17:06:09 PST
*** Bug 510094 has been marked as a duplicate of this bug. ***
Comment 25 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-03-16 07:58:16 PDT
*** Bug 567326 has been marked as a duplicate of this bug. ***
Comment 26 Henrik Skupin (:whimboo) 2011-05-23 01:11:43 PDT
*** Bug 658774 has been marked as a duplicate of this bug. ***
Comment 27 Jez Ng [:int3] 2011-06-26 10:20:37 PDT
Created attachment 542029 [details] [diff] [review]
Patch v1
Comment 28 Jez Ng [:int3] 2011-06-26 10:31:11 PDT
Created attachment 542030 [details] [diff] [review]
Patch v1 [more lines of context]
Comment 29 Dorian 2011-06-26 15:15:20 PDT
@Jezreel : Thanks a lot, finally, someone did it ! :)
Comment 30 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-27 08:18:48 PDT
Comment on attachment 542030 [details] [diff] [review]
Patch v1 [more lines of context]

Review of attachment 542030 [details] [diff] [review]:
-----------------------------------------------------------------

This change will need a ui-review, since you are actually changing the way it works for all users, and removing an option.

I think the original reasoning behind replacing current tab, was that the user was going to replace his current task (the currently selected tab) with a new task (open these links), and I feel like either way we are going to make someone sad. Plus we lose an option:

Current situation:
- left click: replace current (single) tab with multiple tabs
- CTRL + left click: append new tabs (undiscoverable, though)
With the patch:
- always append new tabs

The existing approach is coherent with browser handling and that behavior is used:
- on startup when invoking with links in the command line; this makes sense and a key modifier is uninteresting.
- on clicking the home button (with multiple homepages set); this has already bugs filed, since clicking home button multiple times can create fancy results. For example with 2 homepages, one is always overwritter, the other one keeps getting duped.

Reintroducing a pref doesn't seem the right way as well, we already had one and it was removed long time ago (3.0), to be coherent with browser links handling.

But I'm not against this change, let's just try to make it proper.
If UX team is fine with the new default behavior and thinks losing the possibility to replace current tab is an uninteresting use-case we can proceed.
We may evaluate to add a key modifier to restore the old behavior, with a tooltip like "Hold down SHIFT to replace current tab" to increase discoverability.
Alex?

::: browser/components/places/src/PlacesUIUtils.jsm
@@ +826,5 @@
>        return;
>      }
>  
>      var loadInBackground = where == "tabshifted" ? true : false;
> +    // Don't replace the current tab (see bug 440093)

I'd prefer if as a comment you use a sum-up of the above reasoning, there is no specific need to point to the bug here:
"Multiple links should open in addition to the current set of tabs, since opening them individually would cause that behavior, and it would be odd to change that when acting on multiple items."
Comment 31 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-27 08:22:07 PDT
(In reply to comment #30)

> We may evaluate to add a key modifier to restore the old behavior, with a
> tooltip like "Hold down SHIFT to replace current tab" to increase
> discoverability.

SHIFT was a bad choice here, we already open links in a new window with it. ALT is bad as well, since it causes the menupopup to close. We may probably invert CTRL behavior, though now I guess if that would make sense being the opposite of what browser does.
Comment 32 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-29 08:53:52 PDT
*** Bug 587132 has been marked as a duplicate of this bug. ***
Comment 33 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-29 13:26:15 PDT
Let's create a new group in panorama with the set of tabs being opened.  This operation would then have no effect on the current group.  It's important to switch to the panorama view when this happens though, so that the user sees that both groups are now available.
Comment 34 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-29 13:28:25 PDT
adjusting summary for the new approach to avoid dataloss when opening a new set of tabs.
Comment 35 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-06-29 13:41:10 PDT
(In reply to comment #33)
> Let's create a new group in panorama with the set of tabs being opened. 
> This operation would then have no effect on the current group.  It's
> important to switch to the panorama view when this happens though, so that
> the user sees that both groups are now available.

Maybe I'm getting old, but really? I know in an ideal case the person would (a) be using panorama & (b) have those new tabs be unrelated to the open ones. But my gut reaction is that this is forcing overhead on people that isn't necessary.
Comment 36 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-29 14:34:43 PDT
yeah, the command to open a folder of bookmarks is basically the oldschool panorama.  Users who are still doing that are going to love the concept of tab groups.  If they don't love tab groups, then really no one does, because the feature is pretty much exactly what they need.  Problem is, people become highly habituated to particular patterns, and these users may not have discovered panorama yet.
Comment 37 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-29 16:02:33 PDT
I don't dislike the idea, clearly we should rename the command, it may also improve Panorama discoverability. We may try it in Nightly after FF7 merge, so we have time to collect feedback on the change.
My only fear is that middle-clicking folders/containers may be a bit surprising, while clicking on "Open All in a new Panorama Group" is explicit, middle-click would look like all existing tabs have been lost.
Comment 38 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-29 16:08:06 PDT
>middle-click would look like all existing tabs have been lost.

we would want to switch to tab view, and then show the new group being created, so that the user can also see their existing group.
Comment 39 Dão Gottwald [:dao] 2011-06-29 23:44:22 PDT
This needs to happen only when panorama is being used. It should not happen when the user has no intention to use panorama.
Comment 40 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-30 02:15:49 PDT
>This needs to happen only when panorama is being used.

I think we should replace this feature with triggering panorama.  Currently we have two not incredibly ideal alternatives:

-dataloss (replace existing tab set)
-lots of tabs open (always add to the existing tab set)

panorama ends up being the clean solution to this problem, groups of tabs remain in their groups.
Comment 41 Dão Gottwald [:dao] 2011-06-30 04:36:50 PDT
It's a fallacy to think that heavy tab users generally need tab grouping to live a better life. I for one decided that I don't want to juggle with groups (I never use multiple windows either) and that the overhead of organizing groups doesn't result in a net win for me. My tab set is catch-all; tabs getting added to this is exactly what I expect.
Comment 42 Frank Yan (:fryn) 2011-06-30 04:45:39 PDT
I don't think we should do this (create a new panorama group) either. The user did not necessarily or even likely create the bookmark folder by saving an entire tab group. Why must the folder be opened as a new tab group?

Even if we are trying to unify the notions of bookmark folders, tags, and tab groups, I doubt this is the right way to go about it.
Comment 43 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-30 05:28:48 PDT
I don't use tab groups, but I'm not this negative on the change. Opening multiple related tabs from a bookmarks folder is hardly related to what is in the current session, tab grouping comes as a possible way of separating these tasks, much better than what we have.
I think the most opposition is due to the fact this forces you to manage tab groups, even if you usally don't (like me). I'd be hardly against this if I could easily merge groups. I tried dragging a group over another one to merge them with a simple gesture, but I can't!
My idea would be more than when you are put into tab groups management you should be able to drag-merge the new group to the default one with a single move, that doesn't seem possible now.
Comment 44 Dão Gottwald [:dao] 2011-06-30 05:49:45 PDT
(In reply to comment #43)
> I don't use tab groups, but I'm not this negative on the change. Opening
> multiple related tabs from a bookmarks folder is hardly related to what is
> in the current session, tab grouping comes as a possible way of separating
> these tasks, much better than what we have.

It seems impossible to say that for any given bookmarks folder and any given session. Personally, I don't even think in sessions, since my "session" contains tabs that I opened for wildly different reasons at different points in time (often months ago).
Comment 45 Alex Faaborg [:faaborg] (Firefox UX) 2011-06-30 14:40:31 PDT
>Why must the folder be opened as a new tab group?

The folder is an indication that the user views these tabs as a related collection of things (sort of group-ish), so the alternatives of adding them to the current set, or overwriting the current set have historically not been ideal.

If we want to do this we should change hte text of the command to "open tabs in new group" perhaps even with a panorama icon when we start to role out limited context menu icons.

For the users who want to add to their current group, I guess we could consider an alternate command (which would appear below the more dominant command of opening a new group), but that seems like a less likely use case to me, especially considering that for many releases we would literally overwrite all of your existing tabs when you did this operation.  That behavior likely resulted in a pattern of people thinking of folders as isolated sets of tabs.
Comment 46 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-06-30 14:45:13 PDT
(In reply to comment #45)
> to me, especially considering that for many releases we would literally
> overwrite all of your existing tabs when you did this operation.

Notice that currently (And for many releases) we overwrote only the currently selected tab, not all current tabs.
Comment 47 Jeff Largent 2011-06-30 14:57:50 PDT
(In reply to comment #46)
> (In reply to comment #45)
> > to me, especially considering that for many releases we would literally
> > overwrite all of your existing tabs when you did this operation.
> 
> Notice that currently (And for many releases) we overwrote only the
> currently selected tab, not all current tabs.

This was exactly the reason for the initial bug report.  Reading the chain of comments above it is almost impossible to tell that.  As far as I'm concerned you can close the bug.

I don't even remember what version of firefox it was when I initially reported this.  It's been so long though that I have learned to open a new blank tab before opening a folder of bookmarks in firefox, or just skip that step and use chrome.
Comment 48 Alan Arndt 2011-06-30 15:07:06 PDT
I'd like to add my comments in about usage as well.

I feel I use Tabs a LOT sometimes with so many tabs open they scroll quite a bit.  However, I've never felt the need to play with tab groups.  I have, on occasion wanted to keep a few things separate, and in doing so I just opened another entire window.  Though that's rare.  So personally I don't find parorama that helpful.  I suspect it might be for those who have a lot of web application type pages open.  I don't use web mail, I don't use facebook, etc.  I use tabs to open the next pages I want to look at, they really aren't grouped in any manner.

I never use right click "Open All in Tabs".  I always use middle click on the folder.  Open All in Tabs does replace the current tabs (but only those to the right, not all of them).  Middle click does not overwrite any tabs, and that's the way I like it, that had better not change or it will destroy most of what I've enjoyed about Firefox since tabs were introduced.

The original point of this bug was that there is a setting to say if you want tabs replaced or appended to (browser.tabs.loadFolderAndReplace) - if that's accessible somewhere in the UI still I have no idea.  If the option exists, then the current behavior is BROKEN.  Just FIX the behavior of the command to follow the option.  Don't turn this into a major production about how the UI should be changed.  That's another discussion if you want.
Comment 49 Dorian 2011-07-03 04:38:12 PDT
Guys, you came too far…

I just wanted the "Open all in tabs" open all bookmarks in tabs.

With middle click, or Ctrl+click : it should open in new tabs (and not replace the current one)

With click on "Open all in tabs", it should replace the current tab and add new tabs at the right.

It do it now (in Firefox-trunk) so I think the bug should be close.
Comment 50 Thomas Ahlblom 2011-07-16 02:06:39 PDT
*** Bug 672018 has been marked as a duplicate of this bug. ***
Comment 51 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-07-19 13:43:15 PDT
(In reply to comment #49)
> It do it now (in Firefox-trunk) so I think the bug should be close.

It doesn't behave consistently, and is not discoverable at all, so I don't see how it may be closed.
The fact is that replacing the single current tab makes no sense, it should either replace all or none, and that's what should be figured out here.
Comment 52 Kevin Ar18 2011-07-20 10:44:28 PDT
It seems a bad idea to tie this bug to panorama.

* This is a definite dataloss bug but that needs fixing; we already have a patch (maybe needs a little polish?); let's fix it as is.
* If people think adding on features for panorama would be a good idea, then do that separately as a new enhancement request -- but don't hold up this dataloss bug for that reason.


Regarding Comment 51,
> The fact is that replacing the single current tab makes no sense, it should either replace all or none, and that's what should be figured out here.
I think most people agreed that it should replace none, am I wrong on this?
Comment 53 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-07-26 06:29:15 PDT
Yes, I'm tempted to take that patch and follow-up ina separate bug to investigate Panorama integration.

Unless someone has a clear use-case for willing to replace the current (and just one) tab.
Comment 54 Kevin Ar18 2011-07-26 10:47:05 PDT
(In reply to comment #53)
> Yes, I'm tempted to take that patch and follow-up ina separate bug to
> investigate Panorama integration.

Thanks for considering this.  Anyways, I apologize for the tone/attitude of my last post.

> Unless someone has a clear use-case for willing to replace the current (and
> just one) tab.
I know I saw one person mention they like it this way... but after searching, I just can't find that post again. :(  However, having to close a tab you don't want (instead of it auto-replacing) seems like it would not be toooo much trouble, in exchange for designing the tab system to be more consistent.


Regarding adding tabs, there was one little quirk:
* If you just launched your browser or if you have a blank tab, this will create new tabs and leave you with a blank tab at the beginning.  I think it was suggested maybe replacing a tab if it is blank... but after some consideration, I don't think a special case should be done for blank tabs either.
Regarding not replacing a blank tab: (*) It preserves consistency, which can be very important; it is much easier to reason about and control something that is consistent (*) the user is specifically requesting new tabs, in addition to the tabs they have now (*) maybe they actually want the blank tab left there.
Comment 55 Alex Faaborg [:faaborg] (Firefox UX) 2011-07-28 16:42:18 PDT
We can fix this separately and do the panorama integration in another bug, I don't really care either way.  I do however feel that we should be leveraging panorama to solve the overall data-loss problem and create a new group.
Comment 56 Jez Ng [:int3] 2011-07-31 00:01:37 PDT
Created attachment 549631 [details] [diff] [review]
Patch v2

I've updated the comments in the patch as per mak's review.
Comment 57 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-08-01 17:18:42 PDT
so, comment 54 is right about the fact the only problematic case is when the current tab is on about:blank, we should most likely overwrite only in this case. Shouldn't be hard to do, right? Alex, do you think it's worth doing that?
Comment 58 Alex Faaborg [:faaborg] (Firefox UX) 2011-08-01 17:48:37 PDT
either way is fine
Comment 59 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-08-02 02:27:19 PDT
Comment on attachment 549631 [details] [diff] [review]
Patch v2

Review of attachment 549631 [details] [diff] [review]:
-----------------------------------------------------------------

ok, let's do this to gather feedback, please land it asap.
Comment 60 Jez Ng [:int3] 2011-08-02 10:55:22 PDT
Created attachment 550126 [details] [diff] [review]
Patch v2 [for checkin]
Comment 62 Tim Taubert [:ttaubert] 2011-08-04 05:38:43 PDT
http://hg.mozilla.org/mozilla-central/rev/410af9c91a7b
Comment 63 WildcatRay 2011-08-26 15:09:20 PDT
(In reply to Marco Bonardo [:mak] from comment #57)
> so, comment 54 is right about the fact the only problematic case is when the
> current tab is on about:blank, we should most likely overwrite only in this
> case. Shouldn't be hard to do, right? Alex, do you think it's worth doing
> that?

Am I correct in assuming the implemented patch also does not overwrite an about:blank tab? (This is what I am seeing on Nightly builds.)

Is there a bug filed already to enable the overwriting of an about:blank tab? (From Comment 58, it would appear to not be a difficult change to make.)

If not, should I file one?

Thanks.
Comment 64 Kevin Ar18 2011-08-29 19:38:32 PDT
(In reply to Ray Murphy (WildcatRay) from comment #63)
> (In reply to Marco Bonardo [:mak] from comment #57)
> > so, comment 54 is right about the fact the only problematic case is when the
> > current tab is on about:blank, we should most likely overwrite only in this
> > case. Shouldn't be hard to do, right? Alex, do you think it's worth doing
> > that?
> 
> Am I correct in assuming the implemented patch also does not overwrite an
> about:blank tab? (This is what I am seeing on Nightly builds.)
> 
> Is there a bug filed already to enable the overwriting of an about:blank
> tab? (From Comment 58, it would appear to not be a difficult change to make.)
> 
> If not, should I file one?
> 
> Thanks.

No, please do not file one! :)

(I say that with a smile, but serious as well.)

Sorry, I think there was some confusion about what I was trying to say in Comment #54.  Basically, I was saying "Do not make an exception for blank tabs" ... and then I presented several reasons why.  Unfortunately, Marco misunderstood me (Comment 57) and thought I was saying we should add an exception.

Anyways, thanks for bringing this up again, because I wanted to clear things up.  I would argue that "We should not make an exception for blank tabs. In other words, do not close blank tabs."  This is based on the reasons given in Comment #54:

* Closing blank tabs would make the behavior inconsistent -- and something that works differently under different situations is often very bad for useability.
* People might actually want to keep the blank tab open; they would then be fighting with their browser trying to "think" for them -- and that is much much worse than having to close a tab every now and then.
Comment 65 WildcatRay 2011-08-30 06:10:49 PDT
(In reply to Kevin Ar18 from comment #64)
> (In reply to Ray Murphy (WildcatRay) from comment #63)
> > (In reply to Marco Bonardo [:mak] from comment #57)
> > > so, comment 54 is right about the fact the only problematic case is when the
> > > current tab is on about:blank, we should most likely overwrite only in this
> > > case. Shouldn't be hard to do, right? Alex, do you think it's worth doing
> > > that?
> > 
> > Am I correct in assuming the implemented patch also does not overwrite an
> > about:blank tab? (This is what I am seeing on Nightly builds.)
> > 
> > Is there a bug filed already to enable the overwriting of an about:blank
> > tab? (From Comment 58, it would appear to not be a difficult change to make.)
> > 
> > If not, should I file one?
> > 
> > Thanks.
> 
> No, please do not file one! :)
> 
> (I say that with a smile, but serious as well.)
> 
> Sorry, I think there was some confusion about what I was trying to say in
> Comment #54.  Basically, I was saying "Do not make an exception for blank
> tabs" ... and then I presented several reasons why.  Unfortunately, Marco
> misunderstood me (Comment 57) and thought I was saying we should add an
> exception.
> 
> Anyways, thanks for bringing this up again, because I wanted to clear things
> up.  I would argue that "We should not make an exception for blank tabs. In
> other words, do not close blank tabs."  This is based on the reasons given
> in Comment #54:
> 
> * Closing blank tabs would make the behavior inconsistent -- and something
> that works differently under different situations is often very bad for
> useability.
> * People might actually want to keep the blank tab open; they would then be
> fighting with their browser trying to "think" for them -- and that is much
> much worse than having to close a tab every now and then.

Thanks for the reply, Kevin. However, I must say that I do not understand the logic behind not overwriting a blank tab.

Let me start by giving you my “use case.” At least once daily, I open a group of bookmarks. I do this by first opening a new window in my browser. If I have a page set to open in a new tab or window, I would close it leaving a blank tab. Then, I would open the group of bookmarks where the first one opens in the blank tab and the remaining bookmarks in new tabs to the right of the first one. Now, I have a blank tab at the far left that I end up closing because it is not overwritten like it was prior to this patch.

With this in mind, wouldn’t logic say to answer these questions:

* What reason would anyone have to keep a blank tab open when in the process of opening a group of bookmarks?

* What can someone do with a blank tab other than open something--a bookmarked site, a group of bookmarks, a local file or similar--in it?

* If the user was planning to open something different in that first blank tab, wouldn’t they be more likely to have already opened it in the first place prior to opening the group of bookmarks?

As for what has been done, am I correct in that the only justification for not overwriting a blank tab is for the sake of consistency?

And, am I also correct in assuming that having the browser overwrite a blank tab (about:blank), but not overwrite any other tab would not be a major undertaking?
Comment 66 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-08-30 06:15:01 PDT
I won't oppose if you want to file a bug to override blank tabs, that will require ux evaluation and feedback, but for sure that discussion should not live here.
Comment 67 WildcatRay 2011-08-30 06:51:34 PDT
(In reply to Marco Bonardo [:mak] from comment #66)
> I won't oppose if you want to file a bug to override blank tabs, that will
> require ux evaluation and feedback, but for sure that discussion should not
> live here.

Thanks and no problem for me, Marco. Filed Bug 683146
Comment 68 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 14:46:10 PDT
No longer reproducible in Firefox 9.0a2 20110930, marking verified. Please reopen if you can reproduce this on the latest nightly
Comment 69 John Kitz 2011-10-29 03:12:06 PDT
I reported this for Firefox/4.0.1 in bug report 658774, which was then marked as duplicate of this one which is now marked as resolved. Since then I have gone through all the upgrades offered up to and including 7.0.1 (current release) in which the pbm. still exists.

Comment 68 above states that the pbm. no longer exists in FF 9.0a2. It not a biggy, but will actual resolution of the pbm. in the latest release only occur once 9.x is released or is the module in which the pbm. existed already included the current GA release?
Comment 70 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-10-29 03:21:30 PDT
Check the Target Milestone field.
Comment 71 John Kitz 2011-10-29 04:13:50 PDT
Tnx. I guess I have to do the tutorial on bugzilla fields :)

Again, it's not a big issue as far as I'm concerned, but it would be nice if it would be fixed.

Does the fix also address the behavior of FF when I start FF and click on a URL in another application, such as an email client or an RSS reader, in which case the URL is opened in a new tab, while the empty tab that FF started with remains empty?
Comment 72 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-10-29 04:22:35 PDT
(In reply to John Kitz from comment #71)
> Does the fix also address the behavior of FF when I start FF and click on a
> URL in another application, such as an email client or an RSS reader, in
> which case the URL is opened in a new tab, while the empty tab that FF
> started with remains empty?

no, there is no special detection for empty tabs for now.
Comment 73 John Kitz 2011-10-29 05:38:23 PDT
Tnx. How would I get that resolved? Open yet another bug?

I mentioned it in bug 658774 since I expected that both behaviors are part of the same piece of code, but that bug was marked as duplicate of this bug and you state here that the 2nd behavior is not going to be fixed by the fix that resolves the issue of pushing tabs to the right when opening multiple sites from the bookmarks.
Comment 74 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-10-29 05:48:31 PDT
bug 683146
Comment 75 John Kitz 2011-10-29 06:11:32 PDT
Tnx. and for all the great work that's being done on FF!!
Comment 76 Nicolas Barbulesco 2011-11-03 06:46:33 PDT
(In reply to Kevin Ar18 from comment #64)

> * Closing blank tabs would make the behavior inconsistent -- and something
> that works differently under different situations is often very bad for
> useability.
> * People might actually want to keep the blank tab open; they would then be
> fighting with their browser trying to "think" for them -- and that is much
> much worse than having to close a tab every now and then.

(In reply to Ray Murphy (WildcatRay) from comment #65)

> * What reason would anyone have to keep a blank tab open when in the process
> of opening a group of bookmarks?

> * What can someone do with a blank tab other than open something--a
> bookmarked site, a group of bookmarks, a local file or similar--in it?

> As for what has been done, am I correct in that the only justification for
> not overwriting a blank tab is for the sake of consistency?

I prefer leaving the blank tab intact.
Dicussing that in request bug 683146
Comment 77 John Kitz 2011-11-08 12:15:57 PST
Upgraded to 8.0. The behaviour, although consistent, has IMHO gotten worse rather than better. I now get as many additional tabs as bookmarks in the folder in which I select "Open all in tabs", rather then overwriting the tabs that are already open from the selected tab onwards.
Comment 78 Nicolas Barbulesco 2011-11-09 08:52:02 PST
(In reply to John Kitz from comment #77)

Keeping the existing tab(s) intact is the behaviour I want and expect.

Let's please everyone and have two clear & consistent behaviours. Not mixing them in one confusing behaviour.

1/ Keeping the existing tab(s) intact. Let's call this action "Open all as new tabs" or "Open all in new tabs".

2/ Overwriting the existing tab(s). Let's call that action "Open all using existing tabs".
Comment 79 WildcatRay 2011-11-09 09:02:26 PST
FYI-This discussion should be held elsewhere as this bug is closed.
Comment 80 John Kitz 2011-11-09 11:56:25 PST
(In reply to Ray Murphy (WildcatRay) from comment #79)
> FYI-This discussion should be held elsewhere as this bug is closed.

Noted.
Comment 81 Nicolas Barbulesco 2011-11-10 06:58:04 PST
We continue this discussion in request bug 683146.
Comment 82 Martin van Boven 2011-12-10 23:24:11 PST
Nice.  AGAIN a bug has been willfully introduced in Firefox as the result of people with less intelligence shouting louder.  This is as retarded as opening a /single/ bookmark in a new tab instead of in the existing tab, because of the possibility of "data loss".
At least now I know this willfully introduced bug has a very small chance of getting repaired, as some cattle think it is a repair.

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