961016 - When starting Firefox, Unloaded background tabs don't have icon

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Avi Halachmi (:avih)]

Starting with today's nightly, when Firefox starts, background tabs which are unloaded don't have an icon.

I was told by ttaubert that it could be a regression from bug 942374.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

I'm just going to blame bug 942374 until we know otherwise.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

So yeah, I can 100% reproduce that. Pinned tabs have icons, unpinned have not. The title seems also not set properly, it's just the URL.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

The webNavigation.stop() call in ContentRestore.restoreHistory() overrides the icon set by SessionStore.restoreTabs(). This needs to happen on :restoreHistoryComplete. This does fix the icon but the label is somehow still overriden with the URL...

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

browser.stop() seems to override a DOMTitleChanged event that overrides pending tabs' titles. This also seems to be the cause for all pinned tabs "glowing" after the session has been restored.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

I assume that stopping the tab load right after it has been created suppresses the DOMTitleChanged event for about:blank somehow. The little gap we have by sending a message probably lets the page load far enough to emit that.

Comment 7

[alex_mayorga]

I also see this on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140117030207 CSet: 9bcc52594322 so broadening the platform a bit.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-961016-Fix-icon-and-label-for-pending-tabs-with-.patch

This patch sets a pending tab's icon and label after its history has been restored and the about:blank load has been cancelled. The test ensures that we won't regress this again.

Dao, asking for review on the tabbrowser.xml change that ignores DOMTitleChanged events for pending tabs, as soon as the tab has started loading and we use an async message to call "browser.stop()" there seems to be no way to cancel that event. We don't want that titles for pending tabs change and that seems better than trying to catch the event and stop it.

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8361775 [details] [diff] [review]
0001-Bug-961016-Fix-icon-and-label-for-pending-tabs-with-.patch

Review of attachment 8361775 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Tim.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8361775 [details] [diff] [review]
0001-Bug-961016-Fix-icon-and-label-for-pending-tabs-with-.patch

>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -3186,16 +3186,19 @@
>           if (!event.isTrusted)
>             return;
> 
>           var contentWin = event.target.defaultView;
>           if (contentWin != contentWin.top)
>             return;
> 
>           var tab = this._getTabForContentWindow(contentWin);
>+          if (tab.hasAttribute("pending"))
>+            return;

Seems like you should similarly update the e10s code path in the receiveMessage method.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/6b0f990a4e68

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6b0f990a4e68

Comment 17

[Andreas Wagner [:TheOne] [use NI]]

Verified on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0