Last Comment Bug 595601 - Option to not load tabs from inactive groups on initial browser startup (and until such time as the tab(s) become part of an active group)
: Option to not load tabs from inactive groups on initial browser startup (and ...
Status: VERIFIED FIXED
: footprint, perf, ue
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P1 enhancement
: Firefox 6
Assigned To: Tim Taubert [:ttaubert]
:
Mentors:
Depends on: 586068 597248 636279 650573 676638
Blocks: 591775 597681 604213 653099 658913
  Show dependency treegraph
 
Reported: 2010-09-12 00:37 PDT by IU
Modified: 2016-04-12 14:00 PDT (History)
35 users (show)
mounir: in‑testsuite+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (10.25 KB, patch)
2011-01-22 13:15 PST, Michael Yoshitaka Erlewine [:mitcho]
paul: review-
Details | Diff | Review
Patch v2 (11.58 KB, patch)
2011-01-24 22:17 PST, Michael Yoshitaka Erlewine [:mitcho]
paul: review-
Details | Diff | Review
Annotated test log with patch v2 (1.05 KB, text/plain)
2011-01-24 22:19 PST, Michael Yoshitaka Erlewine [:mitcho]
no flags Details
patch v3 (23.53 KB, patch)
2011-04-23 13:30 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Review
patch v4 (24.85 KB, patch)
2011-05-11 10:18 PDT, Tim Taubert [:ttaubert]
paul: review+
Details | Diff | Review
patch v5 (38.49 KB, patch)
2011-05-17 05:21 PDT, Tim Taubert [:ttaubert]
paul: review+
Details | Diff | Review
patch for checkin (push with bug 636279 and bug 650573) (33.50 KB, patch)
2011-05-19 15:13 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Review

Description IU 2010-09-12 00:37:36 PDT
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre

A problem with current TabCandy implementation is that it loads all tabs -- including those in inactive groups -- into memory, on browser startup.

This, to an extent, defeats a perceived benefit of TabCandy and creates a problematic disconnect between what a user may expect and what is actually happening.

I ran into this issue when I noticed that Firefox was consuming over 400 MB of RAM, but I had only a handful of relatively 'light' tabs loaded.  I thought something was leaking, but when I was able to reproduce the issue in safe-mode, with a single tab, I finally opened "Group Your Tabs" and, lo and behold, I had hidden tab groups that Firefox was all-to-happy to load into memory -- even though they were not being used.

This is quite problematic, because some users, as happened with me, will be under the impression that they can create a whole bunch of tab groups -- that will be accessible to them sometime later when they want -- and not have to clutter and bog down the browser with many tabs, not realizing that the latter will continue to occur.

It would be nice if the default behavior is to load only the tabs in the active group(s), initially, and to have an option so that those who want to always load the whole mess can so choose.


Reproducible: Always
Comment 1 Jo Hermans 2010-09-12 02:19:00 PDT
please search before you file a bug

*** This bug has been marked as a duplicate of bug 561149 ***
Comment 2 IU 2010-09-12 08:20:42 PDT
Not even close to a dupe.  Please carefully before duping.
Comment 3 IU 2010-09-12 08:21:41 PDT
(In reply to comment #2)
> Not even close to a dupe.  Please carefully before duping.

Meant to say: please read carefully before duping.
Comment 4 IU 2010-09-12 08:27:02 PDT
I didn't state it explicitly, but the implication is that those hidden tabs SHOULD NOT BE LOADED AT ALL -- until such time as they first become active (i.e. by being part of an active group).
Comment 5 Ian Gilman [:iangilman] 2010-09-16 15:52:16 PDT
Note that we need to make sure Panorama's thumbnail cache is nice and solid for this, so you can still see the thumbnails for the tabs that haven't been loaded.
Comment 6 Aza Raskin [:aza] 2010-09-16 15:56:36 PDT
One of our core strategic initiatives was to decrease startup time and increase real and perceived speed. Panorama allows you to handle many more tabs that was previously sane, but this also hurts us on restoring the session startup experience. The cascaded loading of bug 586068 helps a lot, but we can go further and get truly zippy'ness by not loaded tabs from hidden groups until we need to.

In zpao's words "It *should* actually be a tremendously trivial change after bug 586068".

I think of this as the ultimate in polish and would like to mark this blocking for beta n, where n is when our dependency on the cascading loading is done.
Comment 7 Ian Gilman [:iangilman] 2010-09-16 16:21:04 PDT
(In reply to comment #5)
> Note that we need to make sure Panorama's thumbnail cache is nice and solid for
> this, so you can still see the thumbnails for the tabs that haven't been
> loaded.

Filed bug 597248 for this.
Comment 8 Mike Beltzner [:beltzner, not reading bugmail] 2010-09-22 14:04:18 PDT
We'll take this if it's ready in time, wouldn't hold release on it.
Comment 9 ithinc 2010-09-26 06:59:34 PDT
Load Tabs Progressively (https://addons.mozilla.org/en-US/firefox/addon/91919/) behaves like the request.
Comment 10 IU 2010-09-26 07:59:44 PDT
(In reply to comment #9)
> Load Tabs Progressively (https://addons.mozilla.org/en-US/firefox/addon/91919/)
> behaves like the request.

Panorama is out-of-the-box functionality.  Only the geekiest of users will connect the dots, understand the problem, and have the inkling to search for an extension that may possibly alleviate the issue.
Comment 11 The 8472 2010-11-26 17:25:48 PST
(In reply to comment #6)
> One of our core strategic initiatives was to decrease startup time and increase
> real and perceived speed.

This isn't just about speed. Like the reporter mentioned it can significantly affect the memory footprint of firefox. Assuming some usage pattern where some groups are used more like bookmarks/"will read this later" or people simply being lazy about closing tabs it will become more common that a large amount of tabs is open. To avoid the impression of bloat those tabs shouldn't take up memory (which can easily range in the gigabytes, as it does for me).
Comment 12 Kevin Hanes 2010-12-10 11:42:10 PST
Moving to b9
Comment 13 Kevin Hanes 2011-01-10 10:07:21 PST
bugspam (moving b9 to b10)
Comment 14 Kevin Hanes 2011-01-10 10:10:06 PST
bugspam (removing b9)
Comment 15 Michael Yoshitaka Erlewine [:mitcho] 2011-01-20 16:45:51 PST
Moving goalpost to beta 11. Paul, do you think you'll get to this at all for fx4?

We most likely will punt on this...
Comment 16 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-20 22:28:05 PST
Not a blocker, so it's not likely I'll get to work on this (though I'd like to)
Comment 17 Michael Yoshitaka Erlewine [:mitcho] 2011-01-21 06:36:02 PST
(In reply to comment #16)
> Not a blocker, so it's not likely I'll get to work on this (though I'd like to)

Feel free to unassign yourself if you'd like someone else to take a look at this.
Comment 18 The 8472 2011-01-21 06:48:55 PST
just to note, setting browser.sessionstore.max_concurrent_tabs to 0 does almost what is requested in this bug, except that it also affects the current group instead of just the background ones.
Comment 19 Ian Gilman [:iangilman] 2011-01-21 16:47:48 PST
Of course I desperately want this, but it's way too late for Fx4
Comment 20 Michael Yoshitaka Erlewine [:mitcho] 2011-01-22 13:15:16 PST
Created attachment 506136 [details] [diff] [review]
Patch v1

This introduces a new pref, browser.sessionstore.restore_hidden_tabs, with default true. Test included. Pushed to try.
Comment 21 IU 2011-01-22 18:48:14 PST
(In reply to comment #20)
> Created attachment 506136 [details] [diff] [review]
> Patch v1
> 
> This introduces a new pref, browser.sessionstore.restore_hidden_tabs, with
> default true. Test included. Pushed to try.

Works very well!  Thanks, Michael.

Tested with three tab groups, and the patch does exactly what this bug requested: Loads only the tabs from the active group.

Used the following try server build: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mmitcho@mozilla.com-d2c31ca776bd/try-w32/

I didn't encounter any problems, but others will have to double-check.
Comment 22 Michael Yoshitaka Erlewine [:mitcho] 2011-01-22 20:37:51 PST
Passed try.
Comment 23 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-24 11:03:58 PST
Comment on attachment 506136 [details] [diff] [review]
Patch v1

I'm glad this was as easy as I'd hoped it would be :)

>+// Whether to automatically restore hidden tabs (i.e., tabs in other tab groups) or not
>+pref("browser.sessionstore.restore_hidden_tabs", true);

That pref name could be bit confusing, but it's a hidden pref so oh well.

If we can get this in for beta 11, it might be good to just set this to false and see how it goes.

>     // Look in visible, then hidden
>     let nextTabArray;
>     if (this._tabsToRestore.visible.length) {
>       nextTabArray = this._tabsToRestore.visible;
>     }
>-    else if (this._tabsToRestore.hidden.length) {
>+    else if (this._restoreHiddenTabs && this._tabsToRestore.hidden.length) {
>       nextTabArray = this._tabsToRestore.hidden;
>     }

This made me realize that returning from PB mode could get a bit weird. Tabs that were loaded would need to be loaded again if they weren't in the active group. Not a big deal though.

>+ * The Initial Developer of the Original Code is
>+ * Mozilla Foundation.
>+ * Portions created by the Initial Developer are Copyright (C) 2010

It's 2011 now!

>+function test_loadTabs(expectHiddenTabs, callback) {
>+  // Set the restore_hidden_tabs pref.
>+  Services.prefs.setBoolPref("browser.sessionstore.restore_hidden_tabs", expectHiddenTabs);
>+
>+  let state = { windows: [{ tabs: [
>+    { entries: [{ url: "http://example.com#1" }], extData: { "uniq": r() } },
>+    { entries: [{ url: "http://example.com#2" }], extData: { "uniq": r() }, hidden: true },
>+    { entries: [{ url: "http://example.com#3" }], extData: { "uniq": r() } },
>+    { entries: [{ url: "http://example.com#4" }], extData: { "uniq": r() }, hidden: true }
>+  ] }] };

You're not actually using any extData checks, so you can get rid of assigning uniq & the definition of r() (which is defined in head.js )or future reference)

>+
>+  let expectedTabs = expectHiddenTabs ? 4 : 2;
>+
>+  let check = function(unrestored) {
>+    gBrowser.tabContainer.removeEventListener("SSTabRestored", onSSTabRestored, true);
>+
>+    let testWindow = Services.wm.getEnumerator("navigator:browser").getNext();

You're not using testWindow.

>+    is(gBrowser.visibleTabs.length, 2, "only 2 visible tabs");
>+    let tabs = gBrowser.tabs;
>+    ok(!tabs[0].hidden, "first is visible");
>+    ok(tabs[1].hidden, "second tab is hidden");
>+    ok(!tabs[2].hidden, "third tab is now visible");
>+    ok(tabs[3].hidden, "third tab is now hidden");
>+
>+    is(restoredTabs, expectedTabs, expectHiddenTabs ?
>+      "We restored all four tabs, including the hidden" :
>+      "We restored only the two visible tabs"
>+    );
>+    
>+    is(unrestored, 4 - expectedTabs, expectHiddenTabs ?
>+      "There are no unrestored tabs left" :
>+      "Two unrestored tabs remain"
>+    );

This is a bit gross. Instead of trying to force the ternary in there, I would make it cleaner and just use if/else & repeat the is

>+    
>+    callback();
>+  };
>+
>+  let restoredTabs = 0;
>+  function onSSTabRestored(aEvent) {
>+    if (++restoredTabs == expectedTabs)
>+      check(needsRestore());
>+  }
>+  gBrowser.tabContainer.addEventListener("SSTabRestored", onSSTabRestored, true);
>+  
>+  ss.setBrowserState(JSON.stringify(state));
>+}

You have a few lines that are just spaces, please trim those.

I'd really like to see a test with more than max_concurrent_tabs # of tabs that are hidden. Then simulate switching groups by hiding & showing and ensure that all of the tabs load. Ideally all of the visible tabs would finish loading before you switched visiblity. I'd want to check that we actually get max_concurrent_tabs restoring after the change (might be a bit difficult to actually check, but I think it's important to do so). We should be fine with the new pref == true, but false is where I'm concerned.

r- until that test exists & is passing. I'm not sure it will pass without more changes to sessionstore itself.
Comment 24 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-24 11:09:52 PST
To be clearer, what I expect is going to happen is that we'll restore 1 tab (since we should get TabSelect which should call restoreTab directly) and then we'll only restore 1 at a time after that. So we might need to add a call to restoreNextTab in onTabShow(). It would cause restoreNextTab to get called excessively but it was designed to handle that.
Comment 25 The 8472 2011-01-24 11:57:35 PST
(In reply to comment #23)
> If we can get this in for beta 11, it might be good to just set this to false
> and see how it goes.

I would advise against that. Setting browser.sessionstore.max_concurrent_tabs = 0 currently breaks switch to tab behavior (see bug 599909), so i assume browser.sessionstore.restore_hidden_tabs = false will do the same.
Comment 26 IU 2011-01-24 12:36:13 PST
(In reply to comment #25)
> I would advise against that. Setting browser.sessionstore.max_concurrent_tabs =
> 0 currently breaks switch to tab behavior (see bug 599909), so i assume
> browser.sessionstore.restore_hidden_tabs = false will do the same.

Why would you assume that?  They don't do the same thing.  Besides, it's a beta and the point is to enable it to see if causes any problems.  If it doesn't get proven in a beta, when do you expect it to be?
Comment 27 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-24 13:05:29 PST
(In reply to comment #25)
> (In reply to comment #23)
> > If we can get this in for beta 11, it might be good to just set this to false
> > and see how it goes.
> 
> I would advise against that. Setting browser.sessionstore.max_concurrent_tabs =
> 0 currently breaks switch to tab behavior (see bug 599909), so i assume
> browser.sessionstore.restore_hidden_tabs = false will do the same.

It would do the same. And yes, I know about that brokenness. I filed & wrote the patch that will fix it :)

(In reply to comment #26)
> They don't do the same thing.

They actually end up doing the exact same thing.
Comment 28 Michael Yoshitaka Erlewine [:mitcho] 2011-01-24 22:17:40 PST
Created attachment 506661 [details] [diff] [review]
Patch v2

Updated test based on your comments.

The restore of all hidden tabs when they become unhidden actually seems to be working. I edited onTabShow first, as you suggested, but when I removed that later to check, it actually passed that part.

I'm using your countTabs function, now, and check the isRestoring value after every SSTabRestored. I'm not sure if it's because of this timing, but there's one test failure for me, at least locally. It's in the first test (where we start restoring all four tabs). The first isRestoring check fails, as it says there are two tabs being restored at that point... max_concurrent_tabs of course is set to 1. Am I misunderstanding something about this metric?

I'll post log output as well.
Comment 29 Michael Yoshitaka Erlewine [:mitcho] 2011-01-24 22:19:22 PST
Created attachment 506663 [details]
Annotated test log with patch v2
Comment 30 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-24 23:14:49 PST
(In reply to comment #28)
> Created attachment 506661 [details] [diff] [review]
> Patch v2
> 
> Updated test based on your comments.
> 
> The restore of all hidden tabs when they become unhidden actually seems to be
> working. I edited onTabShow first, as you suggested, but when I removed that
> later to check, it actually passed that part.

Sorry I should have been clearer. We need a test for what would be the default: max_concurrent_tabs == 3 (or really anything greater than 1). When the max is 1, that's not going to actually test the potential problem I tried to describe above. With max_concurrent_tabs == 1, I would expect your test to always pass.

> I'm using your countTabs function, now, and check the isRestoring value after
> every SSTabRestored. I'm not sure if it's because of this timing, but there's
> one test failure for me, at least locally. It's in the first test (where we
> start restoring all four tabs). The first isRestoring check fails, as it says
> there are two tabs being restored at that point...

Probably a timing issue. countTabs was setup to count correctly based on the order the tabsProgressListener was added in relation to when sessionstore's was added and the order they would get notified of events. You might need to do what I did in browser_586068-cascaded_restore.js with the progress listeners to get a more accurate count.
Comment 31 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-01-26 09:55:07 PST
Comment on attachment 506661 [details] [diff] [review]
Patch v2

r- based on comment 30
Comment 32 Michael Yoshitaka Erlewine [:mitcho] 2011-02-01 11:02:34 PST
Based on our limited time and capacity for fx4, as well as the slight risk that this patch poses, we're going to punt on this for fx4.
Comment 33 Michael Yoshitaka Erlewine [:mitcho] 2011-03-23 11:40:14 PDT
I'm moving on from Panorama work now, so I'm going to leave this bug open now. The core of the patch is working, though, so if someone can take this the extra mile based on zpao's comments, that would be great and a welcome addition to fx5!
Comment 34 Kevin Hanes 2011-03-31 10:51:14 PDT
bugspam
Comment 35 Tim Taubert [:ttaubert] 2011-04-23 13:30:48 PDT
Created attachment 527960 [details] [diff] [review]
patch v3

Passed try:

http://dev.philringnalda.com/tbpl/?tree=Try&pusher=tim.taubert@gmx.de&rev=72ca7ab9cca0

Explanation for changes to browser_tabview_bug595560.js:

This test (right before the actual test belonging to this bug) created a group but didn't delete it afterwards. So that was the permanent Win XP opt only orange.
Comment 36 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-05-10 16:13:21 PDT
Comment on attachment 527960 [details] [diff] [review]
patch v3

I suck at reviewing in a reasonable amount of time... I'm sorry. But I'll be much more on top of this from here out.

Now correct me if I'm wrong (please, I would love to be wrong here), but doesn't this suffer from the same problem that I pointed out when reviewing Raymond's patch? I didn't see anything explicitly checking that we were restoring 3 tabs after switching groups, only < 4. The changes to nsSessionStore don't include a call to restoreNextTab in onTabShow (which adds to my suspicions).

So again, here's the situation I'm concerned about:
* 4 visible tabs, 4 hidden tabs (you have that)
* we get to < max_concurrent_tabs left restoring/need restore. I think even 0 is going to break.
* we switch groups
* we're not restoring max_concurrent_tabs at a time because we don't call restoreNextTab enough.

Am I crazy or is that still not properly tested?
Comment 37 ithinc 2011-05-10 23:19:16 PDT
(In reply to comment #23)
> >+// Whether to automatically restore hidden tabs (i.e., tabs in other tab groups) or not
> >+pref("browser.sessionstore.restore_hidden_tabs", true);
> 
> That pref name could be bit confusing, but it's a hidden pref so oh well.

I second this. Maybe it would be better to trim the "_tabs"?
Comment 38 Tim Taubert [:ttaubert] 2011-05-11 10:18:52 PDT
Created attachment 531667 [details] [diff] [review]
patch v4

(In reply to comment #36)
> So again, here's the situation I'm concerned about:
> * 4 visible tabs, 4 hidden tabs (you have that)
> * we get to < max_concurrent_tabs left restoring/need restore. I think even
> 0 is going to break.
> * we switch groups
> * we're not restoring max_concurrent_tabs at a time because we don't call
> restoreNextTab enough.
> 
> Am I crazy or is that still not properly tested?

Oh snap, thanks for catching that. When I split the test case into two tests I forgot to include it in the tabview test. It of course does fail (and now it totally makes sense to me, too) without the restoreNextTab() call. I removed it while writing the patch because the test passed without it (surprise :).

So browser_tabview_bug595601.js does now check if three tabs are restored concurrently. A restoreNextTab() call was added to onTabShow(aTab) when <aTab> switches visibility.
Comment 39 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-05-11 14:43:15 PDT
Comment on attachment 531667 [details] [diff] [review]
patch v4

Good to know I wasn't crazy :) Looks good to me now.

>diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js
>+// Whether to automatically restore hidden tabs (i.e., tabs in other tab groups) or not
>+pref("browser.sessionstore.restore_hidden_tabs", true);

Let's just go ahead and set this to false and get it landed. We can always set it back to true later if we have to.
Comment 40 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-05-11 16:50:09 PDT
Just in case anybody wanted to jump the gun and land this, don't. Tim pointed out there's orange on his try push with this patch. I'm inclined to say it's the test, but it needs to be looked at more (and running solo passes). I'll take a closer look tomorrow.

http://tbpl.mozilla.org/?tree=Try&rev=5a372382dd16
Comment 41 Tim Taubert [:ttaubert] 2011-05-17 05:21:04 PDT
Created attachment 532931 [details] [diff] [review]
patch v5

Finally passes try (yay!). I had to modify some tests and some head.js functions to deal with hidden tabs not getting restored by default. I modified some tests to execute with restore_hidden_tabs on and off. Therefore asking for review again.

http://tbpl.mozilla.org/?tree=Try&pusher=tim.taubert@gmx.de&rev=134834b69989
Comment 42 Tim Taubert [:ttaubert] 2011-05-17 05:37:46 PDT
Oh and there was some legacy lines of code that counted restored tabs as currently restoring. That seemed to happen only on try and that was the error we saw (4 tabs not 3 tabs restoring).
Comment 43 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-05-18 13:25:40 PDT
Comment on attachment 532931 [details] [diff] [review]
patch v5

Review of attachment 532931 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 44 Tim Taubert [:ttaubert] 2011-05-19 15:13:31 PDT
Created attachment 533810 [details] [diff] [review]
patch for checkin (push with bug 636279 and bug 650573)
Comment 45 Daniel Holbert [:dholbert] 2011-05-19 17:55:33 PDT
(In reply to comment #44)
> patch for checkin (push with bug 636279 and bug 650573)

(volkmar pushed those other bugs' patches to cedar already)

pushed to cedar: http://hg.mozilla.org/projects/cedar/rev/75fc2600eea8
Comment 46 Mounir Lamouri (:mounir) 2011-05-20 07:03:41 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/75fc2600eea8
Comment 47 George Carstoiu 2011-07-05 02:53:03 PDT
Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110704 Firefox/7.0a1

Verified issue on Ubuntu 11.04 x86 and WinXP using the following steps to reproduce:
1. Open Firefox with clean profile
2. In options(preferences) menu set When Firefox starts: "Show my windows and tabs from last time"
3. Go into Panorama and create another group and populate it with all the tabs from BBC's latest headlines
4. Open Task manager and observe the memory consumption
5. Switch to initial group (that has only a few tabs)
6. Close Firefox and watch how memory consumption goes down
7. Start Firefox - memory consumption is low considering that only the previous group is loaded
8. Switch to group with BBC headlines and observe that memory starts going up.

Setting status to Verified Fixed.
Comment 48 zebul666 2012-02-08 07:39:53 PST
is it me or the implementation of the fix is buggy ?

running firefox 10, browser.sessionstore.restore_hidden_tabs was set to false by default and pinned tabs were automatically loaded anyway ? so *** ?
Comment 49 Roman 2012-02-08 07:46:01 PST
That's a feature. A setting to change that (restore_pinned_tabs_on_demand) will be available in Firefox 12, apparently (bug 708585)
Comment 50 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-02-08 10:57:44 PST
(In reply to zebul666 from comment #48)
> running firefox 10, browser.sessionstore.restore_hidden_tabs was set to
> false by default and pinned tabs were automatically loaded anyway ? so *** ?

Pinned tabs are not in any group and are always visible. That pref doesn't apply to them. As Roman mentioned, there was a different bug, now fixed, that will add an additional switch for pinned tabs.
Comment 51 zebul666 2012-02-09 00:59:03 PST
+1 more pain again
for something working fine, now we have to wait until firefox 12....
hoping it will work then...

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