ithinc pointed out that we never set currentURI on the browser until a tab is actually restored (bug 597757 comment 17). This is at least partially responsible for hiding those tabs from switch-to-tab (switchToTabHavingURI looks at browser.currentURI). Setting currentURI as ithinc suggested should fix that problem, but it might not add the tab to matches as you search. I'm not sure where the urlbar looks for matching tabs, so there might be something additional to do there. For _most_ people this won't be an issue, but for people on slow connections or people who have tweaked some prefs, it does become an issue (set browser.sessionstore.max_concurrent_tabs = 0 :/)

The in-memory table that's used to populate "switch to tab" results is built with calls to registerOpenPage/unregisterOpenPage - the tabbrowser takes care of doing this from onLocationChange. I suppose session history could just do that too...

(In reply to comment #1) > The in-memory table that's used to populate "switch to tab" results is built > with calls to registerOpenPage/unregisterOpenPage - the tabbrowser takes care > of doing this from onLocationChange. I suppose session history could just do > that too... So setting current uri via browser.docShell.setCurrentURI(uri) works. That triggers onLocationChange.

(In reply to comment #2) > So setting current uri via browser.docShell.setCurrentURI(uri) works. That > triggers onLocationChange. Does anybody know if doing this is going to have effects elsewhere? It seems like an ok solution, but anytime I call directly into docShell I get a little uneasy. calling registerOpenPage directly might work, but switchToTabHavingURI checks each browser.currentURI which I would assume to be wrong if the tab hasn't been restored yet (we wouldn't have called sessionHistory.gotoIndex so I assume currentURI is about:blank)

if you directly call registerOpenPage it will never get unregistered (unless you also do that on tab close).

and it could also be registered to be open twice once the user visits the tab and the page really loads.

> I get a little uneasy. You and the rest of us. ;) I _think_ this should be ok as long as you test things with named anchors... I'm worried about those breaking.

(In reply to comment #6) > I _think_ this should be ok as long as you test things with named anchors... > I'm worried about those breaking. In what way might those break? Assertions? Explosions? I'd like to make sure my tests are actually testing the right thing.

> In what way might those break? Not load the page when the user switches to the tab, say (because we usually treat a load of a URI that matches mCurrentURI up to anchor as meaning "scroll to this anchor", not "load this page").

So setCurrentURI works in practice, however the tests hang. But if I do some trickery Olli suggested in bug 597315 - set index in shistory & call reload instead of gotoIndex - the tests actually finish. (https://bugzilla.mozilla.org/attachment.cgi?id=489848&action=diff#a/browser/components/sessionstore/src/nsSessionStore.js_sec6) Either way, passing or timing out, there's an assertion that I'm mostly certain wasn't there before. I'm certainly not going to say we should ignore that, especially since I'm setting currentURI to something besides about:blank while session history is probably assuming we're still on the transient viewer. ###!!! ASSERTION: no SHEntry for a non-transient viewer?: 'IsAboutBlank(mCurrentURI)', file /Users/pao/mozilla/mozilla-central-git-dev/docshell/base/nsDocShell.cpp, line 9256 Olli? Sounds like you're the one I should be asking about SHistory, so do you have any more work-arounds here? Boris, if you've got anything, feel free to chime in too :)

It sounds like all the to-be-reloaded and switch-to-tab handling should happen somewhere in toolkit/browser, not necessarily depend on docshell so much.

Went with what Olli and I settled upon over IRC. We set the currentURI to the uri we're going to restore when setting up the tab, then set it back to about:blank right before we restore that tab.

I probably should have run all of the sessionstore tests before asking for review... This breaks at least one test (bug 514751) due to trying to create a URI from null. Should be easily avoided: > if (uri) > setCurrentURI... but in those extremely rare cases we won't have a switch-to-tab entry for that tab (that's ok I think)... There seems to be some other issues when run in the suite but I think running the failing tests on their own they do ok... looking into it.

(In reply to comment #11) > Went with what Olli and I settled upon over IRC. We set the currentURI to the > uri we're going to restore when setting up the tab, then set it back to > about:blank right before we restore that tab. Yeah, this is right what Load Tabs Progressively and BarTab behave.

This is also causing browser_522545.js to fail, which is a problem with the test in this case. That test is looking at onLocationChange to determine our restore state. Setting currentURI results in triggering onLocationChange (it's a feature!) and with this patch we end up having 3 onLocationChange calls intead of 1 so we need to do the restore check a different way (one of these days all of these test will use THE ONE WAY)

Very minor code change, but I moved the test into it's own file and reworked it a little bit. This is built on top of the patch (that will be) in bug 614089, but I don't know that it explicitly needs it. The test here still fails until bug 558321 is fixed. One of the private browsing session restore tests closes a tab that was open before entering PB mode, so there's a URL in the switch-to-tab db that shouldn't be there. Since this compares all entries in both directions (db <--> open tabs) the test fails with that extra entry - the open tabs check is fine.

Notes from the Grand Retriage: Candidate for branch, but if we get this into a happy spot in the ff4 cycle, a=me.

Updated so that it can actually land. I had to change the test a bit so it would run (I think we got rid of temp tables so the test as is wasn't running). I just mirrored some changes that went into the test I stole the results checking from and all is good again.

Pushed http://hg.mozilla.org/mozilla-central/rev/30d746c93932

Hi, I don't know the status of this bug, but the problem still exist in the todays nightly 16/2/2011. Regards,

(In reply to comment #21) > Hi, > > I don't know the status of this bug, but the problem still exist in the todays > nightly 16/2/2011. > > Regards, What are your steps to reproduce?

1- Set "browser.sessionstore.max_concurrent_tabs" to "0" in about:config 2- Try to access a tab that's not loaded yet through "switch to tab" feature, it will not appear in "switch to tab" list at all!!! that way i will not benefit from "tab-restore-on-demand (BarTab)" feature !!

Bara'a, it works for me with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre. Can you confirm you are using the same build? Do you have any add-ons installed which might alter tab or session store behaviour?

My build details are : Build identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre and no add-ons installed that related to tabs or session store behavior, i just have the "Home-Dash" Add-on https://addons.mozilla.org/en-US/firefox/addon/prospector-home-dash/ and i disabled it but the status still the same.

Hi Again, I'm a bit confused, i re-try the same scenario again, and it seems that some tabs appear in the "switch to tab" Menu and the remaining are not! but with no obvious pattern! Although i have more than 40 tabs in my session, when i type % in the awesomeBar after a restart, i got a 12 tabs, some of them are pinned as an "App Tab" and some are not!

Maybe 12 is the upper limit for the number of displayed entries.

(In reply to comment #27) > Maybe 12 is the upper limit for the number of displayed entries. Yes, there is a limit of 12 for autocomplete.

Maybe, but even with out using %, if i search for any other tab that's not loaded yet it will not appear with "switch to tab" option.

(In reply to comment #29) > Maybe, but even with out using %, if i search for any other tab that's not > loaded yet it will not appear with "switch to tab" option. Can you try starting Minefield with a new profile and try again? It might be a tab limitation that we are running into. Can you try with 5, 10, 20, then 40 tabs and see what your results are? Thanks

To clarify things a bit, if i have 50 tabs, and 10 of them from Wikipedia, and i try to search for these tabs i will not find anyone. But if i type % Wikipedia all the tabs related to Wikipedia will appear. Thanks,

Dear Anthony, i think you are right, if the autocomplete limit is 12 and i search for all the Wikipedia pages in my history, favorites, and tabs, then it seems that the awesome bar get the first 12 results.

Right, the limit is 12, as designed. I'd be more concerned if you weren't seeing ANY switch-to-tab results for unloaded tabs. The premise of this bug was that no unloaded tabs were appearing in switch-to-tab results. There might be add-ons which increase that 12-count limit; I can't think of any off the top of my head. Thanks for your help on this.

Mhh, i get that behavior too, it seems counter-intuitive that some random history-entries get a higher priority in the autocomplete ordering than the only open tab matching that search. I think instead of providing only 12 results it should provide only N results from each category (bookmarks, switch to tab, history, instant search [if installed via extension], etc.)

(In reply to comment #34) > Mhh, i get that behavior too, it seems counter-intuitive that some random > history-entries get a higher priority in the autocomplete ordering than the > only open tab matching that search. I think instead of providing only 12 > results it should provide only N results from each category (bookmarks, switch > to tab, history, instant search [if installed via extension], etc.) That's a valid concern and should be filed in a separate bug as an "enhancement". Would you mind filing it? Thanks.

ok, filed under bug 634681

Test case added to Litmus: https://litmus.mozilla.org/show_test.cgi?id=15107 Paul, can you please review it? Thanks.

(In reply to comment #37) > Test case added to Litmus: > https://litmus.mozilla.org/show_test.cgi?id=15107 > > Paul, can you please review it? Thanks. Looks good. Thanks!

Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre