261517 - Opening a grouped bookmark focuses first tab in the group regardless of 'browser.tabs.loadInBackground' pref

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Prognathous]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a4) Gecko/20040924

When a grouped bookmark is loaded and browser.tabs.loadInBackground is set to
true, tabs should load in the background. Instead, the first tab in the group is
focused.

Steps to reproduce:
1. Create a grouped-bookmark
2. Under Preferences -> Navigator -> Tabbed Browsing, set the following:
   Uncheck "Select new tabs opened from links" (browser.tabs.loadInBackground =
true)
   Under "When opening a bookmark group", select "Add tabs"
3. Open the grouped-bookmark

Actual Results:  Tabs in the group are appended to existing tabs, and the first
tab of the group is switched to.

Expected Results:  Tabs in the group are appended to existing tabs, and the
current tab is still focused.

The same problem was fixed in Bug 152625 two years ago, but has now returned as
a 1.8 trunk regression. 

Prog.

Comment 1

[Jason Bassford]

Yes, I'm seeing the same thing here.  (I never noticed because I've always had
the preference set to replace rather than append.)  I'm still currently using a
9/19 build so this is at least a week old.

Comment 2

[Gabriel Seah]

I'm seeing this behavior on my build too - 2004091704

Comment 3

[Yuliy Pisetsky]

The pref used by the code for opening a bookmark set (see
http://lxr.mozilla.org/aviarybranch/source/browser/components/bookmarks/content/bookmarks.js#615)
 is using the hidden pref browser.tabs.loadBookmarksInBackground, rather than
browser.tabs.loadInBackground, which is used for links from web pages, rather
than bookmarks.

Comment 4

[Hermann Schwab]

bugs related:

Bug 162189 Ctrl+Enter in URL bar ignores "Load links in the background" pref

Bug 207678 "load link in background tab" pref ignored when opening from personal
toolbar context menu

Bug 216152 Make Bookmarks Manager respect my preference "Open links in the
background"

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a4) Gecko/20040924

I prefer appending of groups loading in the background, so my prefs are:
browser.tabs.loadFolderAndReplace user set boolean false
browser.tabs.loadInBackground     user set boolean true

I´ve set the hidden pref browser.tabs.loadBookmarksInBackground to true,
but it seems this is implemented in the aviary branch only.

If in a webpage I middle-click a link, or right-click open in new tab,
the link is loaded in background, and focus stays were I am.

If I´m loading an URL from the bookmarksmanager, loading in a new tab, it is
loaded in a new tab, but focus switches, so no loading in the background.
Same for the other bugs mentioned.

Mozilla/5.0 (Windows; U; Win98; rv:1.7.3) Gecko/20040911 Firefox/0.10
if I´m using the append group, loading int the background setting in Firefox,
Firefox loads the group in tabs loading in the background, and focus stays were
it was.

So this bug is Mozilla Suite only. 

Comment 5

[Sander]

This regressed between 20040718-09 and 20040726-09. (Can't narrow it down
further, as archive.mozilla.org doesn't hold any more windows builds form
between those dates.)

Looking at those dates, I see the big patch for bug 72361 and related. Testing,
I see that ctrl-click on bookmark groups opens them in the background, so maybe
this regression is even intended? (If so, however, the change isn't consistent,
as a bookmark group set as homepage always opens in the background.)
Cc-ing the responsible parties for clarification / marking this invalid as intended.

Comment 6

[Wladimir Palant]

Yes, this is a regression from bug 72361, I missed this special case.

-> Bookmarks

Comment 7

[Wladimir Palant]

Attachment: Patch

Comment 8

[Wladimir Palant]

Comment on attachment 162285 [details] [diff] [review]
Patch

The patch is somewhat ugly but I can't think of any better solution right now.
Neil, what do you think?

Comment 9

[[e-mail]]

Comment on attachment 162285 [details] [diff] [review]
Patch

I don't like this mixture. I can see three alternatives one of which I hope
you'll like. First, you can just revert clicking to the previous code (so that
shift+clicking a bookmark group won't toggle the background state), basically
this means removing gBookmarkEvent from this patch (but I think your
aTargetBrowser test is a bit cryptic and you should try to simplify it).
Second, you could enhance the target to include a "background" flag which only
affects loading bookmark groups (although you'll have to fix the other code to
treat it as "current"). Third, you could get rid of "tab_background" and set
gLoadInBackground (or whatever) instead. Oh, and you don't need to use if
(typeof aEvent == 'undefined') as if (!aEvent) will suffice.

Comment 10

[Wladimir Palant]

Attachment: Patch v2

1. Ignoring Shift here would be inconsistent.
2. A background flag in the target would be the best solution but this would
require too many changes.
3. If you like gLoadInBackground better then gBookmarkEvent - fine, I've done
it this way.

Btw. if (!aEvent) will produce a JS strict warning for an undefined value...

Comment 11

[[e-mail]]

Comment on attachment 162315 [details] [diff] [review]
Patch v2

By the way, didn't you have a patch for loading tabs from History in the
background?

Comment 12

[Wladimir Palant]

Yes, it is still waiting for superreview...

Can you check this in?

Comment 13

[[e-mail]]

Well, this patch has just bitrotted that one...

Comment 14

[Hermann Schwab]

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