(May be Shell Integration... not sure) Think this may only be the case for Multi window sessions. 1. File Exit 2. Open browser 3. Note title is 'Minefield' until selecting a diff tab (Seems to not happen in the active window but this may be a auth. required prompt doing the tab changing and hiding the symptom) pretty sure this has regressed at some point but not exactly sure when (fairly sure it was the case before Cascaded Session Restore arrived though)

Thanks Charles. If you'd be willing to help find a regression range, that would be helpful!

hmm, do you have about:home in your list of tabs?

?me... nope Paul, I'll try and hunt down a range at some point over the w/e

Ignore that last comment, this is it here, this shows the problem. I have an example 2 windows after session restore using save&quit. Example shown in the screenshot: 1 window has 2 tabs in the background on session restore from "save & quit", the other foreground window has 1 tab. The background window didn't update the titlebar with the page title, but after switching to next tab and back, it shows up. The foreground window updates the page title.

Here's the regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=46310d87f848&tochange=a73c063e52cb

Could be because I explicitly set the tab title right after the load starts (haven't tested though) http://hg.mozilla.org/mozilla-central/file/1c33f957836a/browser/components/sessionstore/src/nsSessionStore.js#l2532 Looks like tabbrowser only calls setTabTitle(tab) after a load completes if the tab title was previously the loading string. Perhaps we shouldn't force a tab title when loading like I did (especially since the <title> might have actually changed).

I think I'm seeing this too. I just updated to the latest Mac nightly (71e8b5aee972), and all of the tabs one of my two windows all have the correct favicon, but their titles are set to "New Tab", and they aren't actually loaded. When I switch to them, they load and the title gets corrected. I don't have the any non-default "*session*" prefs, apart from privacy_level, so I think this bug is more than just failure to set the title.

Hmm, I just updated again and my session restore properly this time... given the lack of changes between this builds, maybe that means the bug is intermittent? :( http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=71e8b5aee972&tochange=6eb42326798b

Please disregard my comments here - I misunderstood what this bug was about!

just call aWindow.gBrowser.updateTitlebar() once after updating all labels or when set selected tab label

I think we should instead add an aTitle parameter to setTabTitle that is preferred over browser.contentTitle, and then have sessionstore just call that.

This also breaks titles in aero peek on windows 7.

(In reply to comment #12) > This also breaks titles in aero peek on windows 7. ...and I'm informed that that is not this issue

added aTitle parameter to setTabTitle and then used it. There's another place in sessionstore where we might want to use this (http://hg.mozilla.org/mozilla-central/file/ff37d20b4fc7/browser/components/sessionstore/src/nsSessionStore.js#l2413) but I think we should actually be ok. If we get into that if block, then we'll get into this block in restoreTab so it's not super important. However I think this could lead to an issue if the page's <title> has been updated between saving the session and restoring it. This should actually already be a problem, it's just wouldn't be super common. We'll force the old title via this change (because we only call setTabTitle if the previous title was tabs.connecting) - http://hg.mozilla.org/mozilla-central/file/ff37d20b4fc7/browser/base/content/tabbrowser.xml#l467 - I guess I'll make sure that's actually the case and file.

Even though Shawn was wrong about this breaking aero peek (my bad) we should still block final on it.

Comment on attachment 482723 [details] [diff] [review] Patch v0.1 This looks like it will set crop="end" when it should be "center" for URIs.

(In reply to comment #16) > Comment on attachment 482723 [details] [diff] [review] > Patch v0.1 > > This looks like it will set crop="end" when it should be "center" for URIs. Ah true. If we explicitly set the label to a URI already then yea, that will happen. We could pass aCrop which would set the be used by default instead of "end"? Then we could just pull off what we already set crop to and use that. Or just explicitly reset it again (amounts to pretty much the same)

Does your patch even fix this bug, i.e. does the document have a title at that point? updateTitlebar needs that.

(In reply to comment #18) > Does your patch even fix this bug, i.e. does the document have a title at that > point? updateTitlebar needs that. It indeed does not fix this bug... Why doesn't updateTitlebar (or I guess getWindowTitleForBrowser) use the tab title? That's set to use the page title or URI anyway...

(In reply to comment #19) > (In reply to comment #18) > > Does your patch even fix this bug, i.e. does the document have a title at that > > point? updateTitlebar needs that. > > It indeed does not fix this bug... > > Why doesn't updateTitlebar (or I guess getWindowTitleForBrowser) use the tab > title? That's set to use the page title or URI anyway... As opposed to the tab label, the window title doesn't display the URI, nor does it display "Loading...".

Comment on attachment 482723 [details] [diff] [review] Patch v0.1 Can these lines just be dropped? // gotoIndex will force the "loading" string, so set the title aTab.label = label;

(In reply to comment #21) > Comment on attachment 482723 [details] [diff] [review] > Patch v0.1 > > Can these lines just be dropped? > > // gotoIndex will force the "loading" string, so set the title > aTab.label = label; I'm fine with that. I offered that up in comment #6. UI will get a little busier than it has been since CSR landed, but not really much different than 3.6.

It would be consistent with the throbber as well. Seems like we should do this and file a follow-up bug on properly suppressing the loading indicators if that's really wanted.

From bug 604550: This can be reproduced by restoring only one tab with undo close. With 2 tabs open, ctrl+w, ctrl+shift+t -> no page title in title bar.

Just removing the title setting sounds good to me.

Get rid of the manual label setting completely here.

Comment on attachment 483599 [details] [diff] [review] Patch v0.2 > try { > didStartLoad = true; >- // Get the tab title (set in restoreHistoryPrecursor) for later >- let label = aTab.label; > browser.webNavigation.gotoIndex(activeIndex); >- // gotoIndex will force the "loading" string, so set the title >- aTab.label = label; > } > catch (ex) { > // ignore page load errors It looks like didStartLoad = true; should be outside of the try block. And maybe there should be a didStartLoad = false; in the catch block? Not relevant for this bug, though.

(In reply to comment #28) > Comment on attachment 483599 [details] [diff] [review] > Patch v0.2 > > > try { > > didStartLoad = true; > >- // Get the tab title (set in restoreHistoryPrecursor) for later > >- let label = aTab.label; > > browser.webNavigation.gotoIndex(activeIndex); > >- // gotoIndex will force the "loading" string, so set the title > >- aTab.label = label; > > } > > catch (ex) { > > // ignore page load errors > > It looks like didStartLoad = true; should be outside of the try block. And > maybe there should be a didStartLoad = false; in the catch block? Not relevant > for this bug, though. Ah yea good call. I'll file & fix.

Pushed http://hg.mozilla.org/mozilla-central/rev/7b3cecd25e60

Verified fixed with: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre ID:20101019165901

Verified fixed for comment 25 (bug 604550) STR as well.