I find it quite awkward that the user of SeaMonkey still has to explicitly opt-in to use tabbed browsing, because defaults are to open a new WINDOW on middle click, Control-click, Control-Enter, and in all other cases. I think all these defaults should be changed (and BTW, why are there even four options for almost the same thing?).

This patch should align our defaults for tabbed browsing with the default prefs Firefox exposes. I think nowadays that's the right way to go by default, users who still want windows for everything can still change their prefs. browser.link.open_external vs. browser.link.open_newwindow is quite confusing, esp. as we have a number of SeaMonkey-internal methods going through .open_external settings, but either merging them or making them have consistent meaning is something that should go into a different bug.

What about promoting the tab items over the window items in the menus?

(In reply to comment #2) > What about promoting the tab items over the window items in the menus? Hmm, good question. I think we should discuss that separately, though, as it would change things even for those who are using the same pref values. In any case, I'd like to do that in a followup, if at all. It surely needs some thought and looking into the issue, though.

(In reply to comment #1) > browser.link.open_external vs. browser.link.open_newwindow is quite confusing, > esp. as we have a number of SeaMonkey-internal methods going through > .open_external settings, but either merging them or making them have consistent > meaning is something that should go into a different bug. browser.link.open_newwindow is really only for windows opened by the page. Perhaps those "internal" places using .open_external could be switched to using browser.tabs.opentabfor.middleclick and middlemouse.openNewWindow instead.

Comment on attachment 459531 [details] [diff] [review] v1: align our tabbrowser prefs with FF >-pref("browser.tabs.loadInBackground", false); >+pref("browser.tabs.loadInBackground", true); Why this change? (assuming it's not just to make tests pass) >-pref("browser.link.open_newwindow.restriction", 0); >+pref("browser.link.open_newwindow.restriction", 2); This change I can live with, especially if Ratty finishes his pref pane.

(In reply to comment #5) > Comment on attachment 459531 [details] [diff] [review] > v1: align our tabbrowser prefs with FF > > >-pref("browser.tabs.loadInBackground", false); > >+pref("browser.tabs.loadInBackground", true); > Why this change? (assuming it's not just to make tests pass) I just saw that Firefox has that set by default, and included it to sync up with their defaults. If you don't want that one, I fully understand. "Making tests pass" shouldn't be a reason for default prefs anyhow, the experience we give users by default should be, and I'd guess that a good part of our new users has been using Firefox before. > >-pref("browser.link.open_newwindow.restriction", 0); > >+pref("browser.link.open_newwindow.restriction", 2); > This change I can live with, especially if Ratty finishes his pref pane. Ah, good, I wondered if we have UI prefs for this... Do you happen to know the bug number for that?

Comment on attachment 459531 [details] [diff] [review] v1: align our tabbrowser prefs with FF >-pref("browser.tabs.loadInBackground", false); >+pref("browser.tabs.loadInBackground", true); OK, so r=me without this change. (In reply to comment #6) > > >-pref("browser.link.open_newwindow.restriction", 0); > > >+pref("browser.link.open_newwindow.restriction", 2); > > This change I can live with, especially if Ratty finishes his pref pane. > Ah, good, I wondered if we have UI prefs for this... Do you happen to know the > bug number for that? Might be 378213. Or it might not.

(In reply to comment #6) > (In reply to comment #5) > > Comment on attachment 459531 [details] [diff] [review] [details] > > v1: align our tabbrowser prefs with FF > > > > >-pref("browser.tabs.loadInBackground", false); > > >+pref("browser.tabs.loadInBackground", true); > > Why this change? (assuming it's not just to make tests pass) > > I just saw that Firefox has that set by default, and included it to sync up > with their defaults. If you don't want that one, I fully understand. > "Making tests pass" shouldn't be a reason for default prefs anyhow, the > experience we give users by default should be, and I'd guess that a good part > of our new users has been using Firefox before. "Because Firefox does it" shouldn't be a reason either. If we define defaults, we should either have real arguments or just confess that we're just playing dice. That said, I've always had tabs loading in the background by default but I have no idea what the average user might prefer since I'm far from that. Leaving it like it is cannot make it worse, though.

This supplementary patch fixes up the plugin test where I introduced a forced switch to tabbed browsing recently - clearing the pref fails if that value is set by default, so this patch actually fixes a test bustage the main patch of this bug would cause.

(In reply to comment #8) > "Because Firefox does it" shouldn't be a reason either. Well, it at least means that it would follow a setting a very large numbers of users that are defaulted to tabbed browsing are using and seem to be happy with. That said, Neil has decided we won't change that one for now, and I'll follow that on checkin.

(In reply to comment #8) > > > >-pref("browser.tabs.loadInBackground", false); > > > >+pref("browser.tabs.loadInBackground", true); > > > Why this change? (assuming it's not just to make tests pass) > > > > I just saw that Firefox has that set by default, and included it to sync up > > with their defaults. If you don't want that one, I fully understand. > > "Making tests pass" shouldn't be a reason for default prefs anyhow, the > > experience we give users by default should be, and I'd guess that a good part > > of our new users has been using Firefox before. > > "Because Firefox does it" shouldn't be a reason either. If we define defaults, > we should either have real arguments or just confess that we're just playing > dice. That said, I've always had tabs loading in the background by default but > I have no idea what the average user might prefer since I'm far from that. > Leaving it like it is cannot make it worse, though. Because tabbed browsing is not really usable without this change. With tabs opening in background I can quickly open bunch of links to read after the current page. With this settings left at false I would be forced to constantly switch tabs after every middleclick. Just consider typical patterns of users with tabbing enabled.

(In reply to comment #5) > Comment on attachment 459531 [details] [diff] [review] > v1: align our tabbrowser prefs with FF > > >-pref("browser.tabs.loadInBackground", false); > >+pref("browser.tabs.loadInBackground", true); > Why this change? (assuming it's not just to make tests pass) This was heavily user-tested, I do agree that "open in tab" and defaults should be in background, rather than "switch to _now_" But if you insist this needs to stay out Neil, we can do a followup bug and/or solicit feedback if you like [worse case solicit all of Council to "vote"] ;-)

Comment on attachment 460394 [details] [diff] [review] fixup plugin test You probably should remove the import of Services too ;)

Comment on attachment 459531 [details] [diff] [review] v1: align our tabbrowser prefs with FF > pref("browser.tabs.loadDivertedInBackground", false); >-pref("browser.tabs.loadInBackground", false); >+pref("browser.tabs.loadInBackground", true); Actually this change is fine after all - it doesn't affect the use case I was worried about.

Pushed the main patch (now with background loading still changed) as http://hg.mozilla.org/comm-central/rev/f94de46e5976 and the plugin test fixup with the nit addressed (I should actually read my own comments when acting on them) as http://hg.mozilla.org/comm-central/rev/5babf5a6c400 ...so this is FIXED and we default to tabbed browsing now!

This fixed bug 378213, which was about changing the browser.link.open_newwindow.restriction pref default to 2. We probably don't have a bug on adding UI for that yet.

1. The change causes a problem with Lightning. If you click on the "calendar button" the calendar gets displayed in the background. 2. The setting "browser.tabs.autoHide" should be changed to false, because the "new tab" button and the lightning "calendar button" are invisible when the tab bar is hidden.

(In reply to comment #17) > 1. The change causes a problem with Lightning. If you click on the "calendar > button" the calendar gets displayed in the background. Please file a new bug for that, even though Lightning is a mere add-on (I wonder how you got it to work with trunk as it's firmly broken there) it might be something to look into some time. > 2. The setting "browser.tabs.autoHide" should be changed to false, because the > "new tab" button and the lightning "calendar button" are invisible when the tab > bar is hidden. That's not connected in any way with this bug, and I'd probably even object to this if it was filed with a patch. Still, the discussion belongs elsewhere.

(In reply to comment #16) > This fixed bug 378213, which was about changing the > browser.link.open_newwindow.restriction pref default to 2. We probably don't > have a bug on adding UI for that yet. Filed bug 583625.

Ugh. If nothing else you could ask when creating a new profile and set the defaults as appropriate. I'd submit a bug to back this out but I'm sure it would get a WONTFIX. I only use tabs with very closely related items (such as comparing two bugs) and use windows for everything else.