User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre ( .NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre ( .NET CLR 3.5.30729) I have never been able to close and restart Minefield and have the tab groupings I set restored. When I click on tabs, the screen has the original window and the "how to create windows" screen displayed. I have simply closed and restarted and have gone to "options" and re-set the current tabs as my home page. Neither makes a difference. Restart should at least bring me to my home page tabs ordered the way I set them. Reproducible: Always Steps to Reproduce: 1.Set tab groups 2.Shut down Minefield 3.Restart Actual Results: In Tab Candy screen, a single window with all home screen tabs displays along with the instruction window. Expected Results: My groupings, at least those for my home page tabs, should beretained.

I can confirm this on Mac - Although for me, some tab groupings are remembered (especially when they have a name) but every time I restart I end up with a whole bunch of tabs on their own in a corner (of the Panorama screen).

Lots of users are complaining about this on Firefox 4 Beta 4 via Input http://input.mozilla.com/en-US/search/?product=firefox&sentiment=sad&locale=en-US&date_end=&date_start=&version=&q=group+tabs

I'm unable to reproduce this myself (Mac) on neither beta 4 nor the recent nightlies. JJM, Rick, are you able to reproduce this with a recent build?

Yes. I did some screen shots and put into a pdf, but don't see how to attach. Open Minefield, open Tab Candy. What shows is the initial version with the "How To Organize Your Tabs" group. Make another group. Close Minefield and restart. Get same initial screen. Have tried going back so I can set Home Page as "Use Current" and done it without. Have tried it with and without naming the group. Is there some setting / option I need to set? I have not found one.

Rick, can you check the error console (under the Tools menu)? Is there anything there which looks related? And does restoring a session work for you? (eg: Go to Preferences, set to restore windows and tabs on startup) This could happen if session restore is not turned on, or not able to write to the disk.

I don't see anything in the Error Console that looks like it is related. Where do I find "Preferences"? I've searched every menu option and do not find it.

This will show you how to find Preferences for your OS: http://support.mozilla.com/en-US/kb/Options+window.

There is no "Preferences" in Options. I did just find the "When Minefield Starts Show windows and tabs from last time" instead of "show my home page". Is this what you mean? If so, it does not work. I selected this, grouped tabs, AND opened a new tab. On restart I get my homepage, without the extra tab and Tab Candy gives me the initial screen as always.

This indicates that there's a problem writing the session restore file to disk. Perhaps a permissions error of some kind?

Advice? I am using Minefield because I think Tab Candy is a cool tool. I want it to work for me and, if I am having problems, likely others are too.

Blocking speculatively, as I've heard some (but few) reports of this, and we need to figure out what's causing it. Rick: your comment about "likely others are too" isn't needed. We're trying to work with you to investigate the issue, which nobody here can reproduce. Can you answer the question in comment 9, please?

I was attempting to answer #9. My "Advice" was intended to mean "What steps do I need to take to help figure this out?" I'm not enough of a computer expert to know where to look to identify the permission issue you are asking me to look into. Sorry for the misunderstanding about my response.

In the newest Minefield, session restore's influence on tab candy (or the other way around of course) has improved considerably. When returning to an actively loaded tab, groups are restored as they were. When returning to an about:blank tab, it loads the tab-bar correctly, but the about:blank tab loses its association with the tab group it belongs to. I'll upload 2 screenshots explaining this visually. (And yes, I do use BarTab 2.0)

Screenshot of blank-tab restore in Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b6pre) Gecko/20100905 Firefox/4.0b6pre

Screenshot of NON-blank-tab restore in Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b6pre) Gecko/20100905 Firefox/4.0b6pre (Sorry about the whiteness - client work!)

(And I'm really sorry about all the bugmails..) Switching to a completely new profile does not change the behaviour described in comment #c14. To replicate: 1) Create new profile in Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b6pre) Gecko/20100905 Firefox/4.0b6pre 2) Create two tab groups 3) Load 2 sites in each 4) Add a new, blank tab to a (the first) group 5) Close and restore with loaded tab selected 6) Switch to tab candy overview mode -> Everything should be ok, including the blank tab. 7) Close and restore with blank tab selected 6) Switch to tab candy overview mode -> Blank tab has lost tab grouping.

See Comment # 13. I have not heard back from you with suggestions. Where do I look? I've found the Profile folder and it showed some sub-folders read-only. Changed to remove that. Restarted and found the same read-only flag. Determined, to the best of my ability, that it is Ad-Block that is protecting itself. I have checked other folders / files that appear to be likely suspects and all appear to be read-write. Is there a specific file I should look for that may be read-only when it should not be? I'm on a small network that my wife and I share and I have everything set to share (read-write)among computers, so I'd not expect that there is a permission setting blocking Firefox/Minefield from writing to disk. If you can give me a couple of suggestions, I'll gladly investigate further.

Hi Rick, lots of folks are busy with bugs that are blocking beta 6. Do not worry, this bug is blocking betaN, which means it must be resolved before we can go to RCs, so it will be attended to!

If I can help in any way, let me know what you need me to do.

Things seem to work fine now, tabs (even empty tabs) are restored and loaded in their appropriate groups. Loaded TXT files do seem to lose their title (it just says "new tab"), but once you click on them they load fine. That's for a different bug though... Tested in: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre

Still not working for me.

(In reply to comment #22) > Still not working for me. So which nightly are you using now? Have you updated the nightly lately? You can find out your current version by entering "about:" in the location bar.

I'm currently showing 4.0b7pre. Updates install every time I start Minefield.

There seems to be some regression in the behaviour. As of the last few nightlies, the tab-bar is filled with a few tabs from other panorama groups (but not all others). Clicking the panorama button fixes everything again. That's in: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b7pre) Gecko/20101006 Firefox/4.0b7pre @Rick It's better to post the build identifier (like I've done above), as that gives a more precise Gecko version, which means it's easier to track.

There seems to be enormous overlap with bug https://bugzilla.mozilla.org/show_bug.cgi?id=587368 too; both have diff meta-bugs tracking them.

I believe the majority of cases of users losing all of their data in tab candy is due not to a particular bug, but to them not understanding that tab candy is managed by session restore. They likely are expecting it to be more persistent, like how bookmarks are managed by places. Two factors that make this even more of an issue: -We default to not saving the user's session -We are about to remove the quit dialog box in favor of placing a message on about:home in the blocking bug 592822 (the user has more context to answer the question when they are starting up) With these two changes, users are guaranteed to lose their tab candy groups every time they close the application. As a quick hack to mitigate the confusion caused by this, I propose we place a restore session command in the center of tab view.

What are the exact strings to be added here?

(In reply to comment #29) > What are the exact strings to be added here? Assuming this is implemented as the mockup shows, I think we'll just need "Always restore previous session" for the checkbox. We already have a string for "Restore Previous Session" (since it's in the menu already), but it might be easier to duplicate the string depending on how panorama is doing its strings

bugspam (moving b9 to b10)

bugspam (removing b9)

(In reply to comment #28) > Created attachment 497934 [details] Please note that it is really, really, hard to quit/open Firefox with Panorama showing (you have to be on a Mac, and hit a command key). So this dialog will only be seen after users have had time to muck around.

... and after testing, "restore previous session" doesn't work if used after the user has opened new tabs (when hit from the history -> restore previous session.

(In reply to comment #35) > ... and after testing, "restore previous session" doesn't work if used after > the user has opened new tabs (when hit from the history -> restore previous > session. It should (and does for me). And if it doesn't that's definitely a bug. Please elaborate on how it's not working.

Can this land today? I don't want any strings to land after this week.

(In reply to comment #37) > (In reply to comment #35) > > ... and after testing, "restore previous session" doesn't work if used after > > the user has opened new tabs (when hit from the history -> restore previous > > session. > > It should (and does for me). And if it doesn't that's definitely a bug. Please > elaborate on how it's not working. This was on my machine, and it just didn't do anything. Trying to reproduce today, however, and it's working fine. I'll keep my eyes open, but for now it seems like it was a fluke.

(In reply to comment #38) > Can this land today? I don't want any strings to land after this week. Might be able to get the strings in today, the rest isn't likely.

(In reply to comment #40) > (In reply to comment #38) > > Can this land today? I don't want any strings to land after this week. > > Might be able to get the strings in today, the rest isn't likely. Sorry, I thought this was a different [strings] bug that was mine. This is not that bug though so I don't know why I'm making estimation calls!

Still need to write a test for it.

softblocker = critical

(In reply to comment #43) > softblocker = critical nope. critical = crashes, loss of data, severe memory leak

Comment on attachment 503944 [details] [diff] [review] v1 I couldn't find a way to write a test to setup and enable the "restore last session". I think we need to flag it with "litmus" for manual testing, unless someone has any suggestions?

(In reply to comment #45) > Comment on attachment 503944 [details] [diff] [review] > v1 > > I couldn't find a way to write a test to setup and enable the "restore last > session". I think we need to flag it with "litmus" for manual testing, unless > someone has any suggestions? Nope, that's not testable through normal browser-chrome. You might be able to do something with mozmill, though you'd likely need to add more sessionstore APIs. I'd just flag in-litmus?

(In reply to comment #46) > (In reply to comment #45) > > Comment on attachment 503944 [details] [diff] [review] > > v1 > > > > I couldn't find a way to write a test to setup and enable the "restore last > > session". I think we need to flag it with "litmus" for manual testing, unless > > someone has any suggestions? > > Nope, that's not testable through normal browser-chrome. You might be able to > do something with mozmill, though you'd likely need to add more sessionstore > APIs. I'd just flag in-litmus? Flagged in-litmus?

Comment on attachment 503944 [details] [diff] [review] v1 Raymond has implemented this as designed, but there are a couple of issues with the design... apologies for not catching these sooner. The design mock-up says "we'll likely change the visual style in a separate bug", but given our release timeline, I'm not confident we'll have time for a follow up. * The button is in the center of the window, but it only shows up during "fresh run" circumstances, where the default group fills 2/3 of the window and therefore covers the button. I propose the button should appear in the lower right. * Having the check box in the button is confusing, and also makes mis-clicks very easy. I propose the check box should appear below the button. ui-review to faaborg for confirmation/counter proposal

Are we past the point of being able to add strings? Do the strings we want already exist somewhere?

Comment on attachment 503944 [details] [diff] [review] v1 >+ // Function: readData >+ // Gets the canRestoreLastSession property. >+ canRestoreLastSession: function Storage_canRestoreLastSession() { The function name comment is incorrect. >+ iQ("#restore-session-button").click(function(e) { >+ if (e.target.nodeName == "INPUT") { >+ if (iQ("#restore-session-checkbox")[0].checked) >+ Services.prefs.setIntPref("browser.startup.page", 3); >+ else >+ Services.prefs.clearUserPref("browser.startup.page"); Clicking on the check box label should also toggle the check box, not just clicking on the check box itself. >- // ___ setup observer to save canvas images >- function quitObserver(subject, topic, data) { >- if (topic == "quit-application-requested") { >+ // ___ setup observers to save canvas images and >+ // remove the restore last session button >+ let theObserver = function(subject, topic, data) { >+ if (topic == "sessionstore-browser-state-restored") { >+ self._hideRestoreLastSessionButton(); >+ } else if (topic == "quit-application-requested") { > if (self.isTabViewVisible()) > GroupItems.removeHiddenGroups(); > > TabItems.saveAll(true); > self._save(); > } > } > Services.obs.addObserver( >- quitObserver, "quit-application-requested", false); >+ theObserver, "sessionstore-browser-state-restored", false); >+ Services.obs.addObserver( >+ theObserver, "quit-application-requested", false); > this._cleanupFunctions.push(function() { >- Services.obs.removeObserver(quitObserver, "quit-application-requested"); >+ Services.obs.removeObserver( >+ theObserver, "sessionstore-browser-state-restored"); >+ Services.obs.removeObserver(theObserver, "quit-application-requested"); > }); I realize this was already here, but now that it's getting expanded, it occurs to me that it really belongs in UI._addTabActionHandlers. >+ if (!this._dismissedRestoreLastSessionButton) { >+ if (Storage.canRestoreLastSession()) >+ this._showRestoreLastSessionButton(); >+ else >+ this._hideRestoreLastSessionButton(); >+ } Can't we just do this check once in init() rather than every time we re-enter Panorama? You could get rid of _dismissedRestoreLastSessionButton then, even. >+ if (button[0].style.display == "none") { >+ button.show(); Probably not necessary to check .display. >+ // move to the centre >+ button.css({ >+ top: Math.floor((window.innerHeight - button.height())/2), >+ left: Math.floor((window.innerWidth - button.width())/2), >+ }); This needs to happen in response to window resize as well. >+ if (button[0].style.display != "none") >+ button.hide(); Also not necessary to check .display here. >+ _restoreLastSession: function UI__restoreLastSession() { >+ gWindow.restoreLastSession(); >+ this._hideRestoreLastSessionButton(); Do we need to hide the button here, or should we rely on our session restore observer to do so? I guess it only makes a difference if the session somehow fails to restore and/or fails to notify us. I dunno, I could go either way; just raising the question.

Comment on attachment 503944 [details] [diff] [review] v1 I'm not familiar with the pref and sessionstore calls used here; they seem fine, but adding Dao for feedback in case I'm missing something about their usage.

(In reply to comment #49) > Are we past the point of being able to add strings? Do the strings we want > already exist somewhere? Re-using existing strings is as bad as adding strings. If you're past the point, yes, string freeze was last year. Anything beyond that is meeting harder resistance by the hour.

Addressed the issues in comment #50 I've also moved the button to the bottom right but the checkbox is still inside the button.

Two more thoughts, one for the code: * It should remember what your startup preference was if you turn the check box on, so if you turn it off again, you can set it back to that rather than assuming a specific option. (For that matter, to be really robust, that check box should watch the pref and turn itself on and off if the user changes the value elsewhere; that's probably too much to hope for, though, and not really necessary at this point.) ... and one for the UX: * So if you don't use session restore, but you do use Panorama, this button will come up every single session, right? So every time you start up the browser, you'll have to either explicitly dismiss the reminder or risk accidentally hitting it. Seems unpleasant; it's basically a nag dialog box recast into the page. Given that we're past the string date and that we're still hashing out the UX, I think it may be too late for this feature; I nominate for unblocking.

FWIW, the home page changes are being done in bug 593421 and we're just reusing a string (which I think is going to be fine). The experience is a bit crappy right now regardless, so I filed bug 627642.

(In reply to comment #56) > FWIW, the home page changes are being done in bug 593421 and we're just reusing > a string (which I think is going to be fine). > > The experience is a bit crappy right now regardless, so I filed bug 627642. Thanks for the bug link, Paul. In fact, because of the update to the home page in bug 593421, users will be presented with the "restore" call to action immediately upon restarting. It's not possible for the user re-enter Panorama directly on restart if they have session restore turned off, so they always have to go through the home page. I believe that makes this bug less relevant.

bugspam. Moving b10 to b11

bugspam. Removing b10

(In reply to comment #57) > In fact, because of the update to the home page in bug 593421, users will be > presented with the "restore" call to action immediately upon restarting. It's > not possible for the user re-enter Panorama directly on restart if they have > session restore turned off, so they always have to go through the home page. I > believe that makes this bug less relevant. Less relevant, sure. But not irrelevant. Especially for people who have changed their home page.

Sorry about the delay, I'm going to try to get to this review tomorrow.

>Less relevant, sure. But not irrelevant. Especially for people who have changed >their home page. Yeah, this might be more of a softblocker than a hardblocker with bug 593421, but from an interface perspective we want to give the user a few opportunities to avoid dataloss, so getting this added is still relatively important. >* So if you don't use session restore, but you do use Panorama Using panorama without using session restore seems strange to me, why would users want to put effort into grouping and moving tabs around if they weren't planning for that to persist very long? Either way, I agree that this is kind of a nag dialog, but it will go away as soon as we get bar tab style on demand loading implemented. At the moment it's just a quick ux hack to avoid users losing all of their panorama data. >The design mock-up says >"we'll likely change the visual style in a separate bug", but given our release >timeline, I'm not confident we'll have time for a follow up. The only issue is that I think we are still using the OS X background color on windows, the dark grey on light blue doesn't look that good. But from an interaction perspective, this patch fixes the perceived data loss problem (which is the blocking-ish part). >I propose the button should appear in the lower >right. yep, that's fine >* Having the check box in the button is confusing, and also makes mis-clicks >very easy. I propose the check box should appear below the button. Mostly I just want these to be visually grouped into a single item, and I copied the visual design of the current (singular) buttons purely for consistency and so that the implementation could go faster. If the main command had a hover state that had more of a button affordance, that would help to address that issue. Alternatively we could style it as a hyperlink, but using hyperlinks for actions in chrome instead of navigations in content is a somewhat contested design pattern.

Comment on attachment 505355 [details] [diff] [review] v2 >+ <input id="restore-session-checkbox" type="checkbox"> >+ <span id="restore-session-checkbox-label"></span> Let's make this span a label tag instead, with for="restore-session-checkbox". I think you can get rid of your "restore-session-checkbox-label" special case code in the click handler that way. >+ let checked = iQ("#restore-session-checkbox")[0].checked ; >+ if (checked) >+ Services.prefs.setIntPref("browser.startup.page", 3); >+ else if (Services.prefs.prefHasUserValue("browser.startup.page")) >+ Services.prefs.clearUserPref("browser.startup.page"); Please add a comment saying what 3 means here, and noting that the default is 1. Also, won't the second part of this override a user's previous pref to use the "blank page" option (0)? I don't think we want that, just by virtue of interacting with this button (even if they check it and uncheck it, then ignore it or close it), right? >+ else >+ this._hideRestoreLastSessionButton(); Isn't this hidden by default at this point? >+ _positionRestoreLastSessionButton: function UI__positionRestoreLastSessionButton() { >+ let button = iQ("#restore-session-button"); >+ if (button[0].style.display != "none") >+ button.css({ >+ bottom: 30, >+ right: 30, >+ }); >+ }, 1. Why aren't we just using CSS? I really don't see a need. 2. Do we want it in the bottom right even in RTL? >+ // we need this because "sessionstore-browser-state-restored" doesn't alway always f+ with the move to CSS positioning (unless there's a good reason, of course).

Updated the patch based on #Comment 63

We should target this to 2.0 since obviously it's going to get fixed. We can track stuff better this way. I assume it's Final, not BetaN?

This has l10n impact, so it's not at all clear for which Firefox version we can take it. Final is not on the table, though.

Comment on attachment 506646 [details] [diff] [review] v3 (In reply to comment #62) > >* Having the check box in the button is confusing, and also makes mis-clicks > >very easy. I propose the check box should appear below the button. > > Mostly I just want these to be visually grouped into a single item, and I > copied the visual design of the current (singular) buttons purely for > consistency and so that the implementation could go faster. If the main > command had a hover state that had more of a button affordance, that would help > to address that issue. Alternatively we could style it as a hyperlink, but > using hyperlinks for actions in chrome instead of navigations in content is a > somewhat contested design pattern. Ok, so let's have a box around all three controls (the restore button, the check-box, and the close box), and let's have an additional box delineating the restore button. The outer box shouldn't have a hover state, and the inner should. I guess you can go with the gray we're currently using for the outer box, and use the "undo close group" gray for the inner box. >+html[dir=rtl] #restore-session-button { >+ left: 30px; >+} Need to add right:auto here as well, otherwise the box is stretched across the entire window. You can use the "force RTL" extension to test: https://wiki.mozilla.org/Firefox/Projects/TabCandy/Work#Tools I suppose we need a test for this. Also, it would be nice to have some sort of determination on whether we're going to get approval for this (due to the new strings), before we spend much more time on it.

(In reply to comment #66) > This has l10n impact, so it's not at all clear for which Firefox version we can > take it. Final is not on the table, though. Beta11's on the table though? I agree it shouldn't go in final if it hasn't been in a beta.

We're string frozen since last year.

If we really wanted this, I don't know why we couldn't have just reused the string like we did in bug 593421 (granted, it's in a DTD, not a properties file so that's fun) and not had the checkbox.

[bugspam: mitcho's late-night betaN triage; feel free to comment or reverse]

I'd love to get some more design love on how we can educate users to use session restore to keep their data. This desire (or something analogous) keeps cropping up in bugzilla and in the media. Can we get some updated mock-ups?

bugspam

bugspam

bugspam

bugspam (Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)

(In reply to Kevin Hanes from comment #72) > I'd love to get some more design love on how we can educate users to use > session restore to keep their data. This desire (or something analogous) > keeps cropping up in bugzilla and in the media. > > Can we get some updated mock-ups? Remove ux-wanted and assigned to limi :)