479397 - this.windowToFocus.content null in nsSessionStore.js

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Matthew Cline]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.8pre) Gecko/2009021904 GranParadiso/3.0.8pre ID:2009021904

When starting FireFox I get the following JavaScript error:

Error: this.windowToFocus.content is null
Source File: ///usr/home/matt/graze/browsers/firefox/components/nsSessionStore.js
Line: 1827

Looking at the source I see this:

    if (this.windowToFocus) {
      this.windowToFocus.content.focus(); /* THIS LINE */
    }

Starting in safe-mode makes this go away, but starting in normal mode with the SessionManager extension disabled doesn't it make it go away.

As far as I can tell this is having no effect on restoring my sessions when I start FireFox.

List of extensions:

About Menu 1.3.2
AboutPlug 1.4
Autofill Forms 0.9.5.2
ChatZilla 0.9.84
Console² 0.3.9.1
Controle de Scripts 1.0.3
Go Parent Folder 1.6
GoogleEnhancer 1.69
Greasemonkey 0.8.20090123.1
gui:config 0.3.7
JavaScript Debugger 0.9.87.4 [DISABLED]
JSView 2.0.5 [DISABLED]
Nightly Tester Tools 2.0.2
No Squint 1.93.2.1
QuickJava 0.4.2.1
Session Manager 0.6.3.5 [DISABLED]
Show Parent Folder 1.4
Smart Middle Click 0.5.1.2
Unhide Menubar 0.0.14
United States English Dictionary 3.0.3

Comment 1

[Michael Kraft [:morac]]

I can reproduce this at will with no extensions installed.

Steps to reproduce:
1. Set Firefox to restore windows and tabs from last time.
2. Have one browser window open and go to any web page.
3. Open another browser window with a blank tab (about:blank).
4. On the new window do a File->Exit.

When Firefox starts up the next time, the error will occur.

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Could we just change that line to the following?

> if (this.windowToFocus && this.windowToFocus.content) {

Or put a try+catch to silently ignore that error? I'm not sure which has more overhead. In this case there's nothing to do about not having any content to focus, so we should just keep moving along.

Simon?

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

I've been seeing this a lot because I'm restoring a session with a focused blank tab, so I wrote the patch. The error's gone. Do we need a test here?

Comment 4

[Simon Bünzli]

Comment on attachment 396775 [details] [diff] [review]
Patch v0.1

This isn't automatically testable, since a restart is required for reproducing. Not sure whether a Litmus test is needed, though.

Comment 5

[Michael Kraft [:morac]]

Attachment: Session Data which generates the exception when restored

A restart isn't necessary to test this.  Simply manually construct the session data like is done for any other SessionStore test.  Then restore it using the setBrowserState API function.

The attached session data will generate the exception when passed as a parameter to setBrowserState.

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #5)
True that a restart isn't needed to make this happen. But it's not possible to catch the error since the call to restoreDimensions is happening in a setTimeout. So the error doesn't propagate up in any sane way, even though we see it in the error console.

We could possibly get all errors bubbling up through Firefox & make sure none of them are specifically this one, but it hardly seems worth the effort.

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/81a206f70733

Comment 8

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 396775 [details] [diff] [review]
Patch v0.1

approval1.9.2? to have one less error in the console.

Comment 9

[Dão Gottwald [:dao]]

I'm not sure I understand this bug. content usually isn't null for about:blank, is it?

Comment 10

[[e-mail]]

(In reply to comment #9)
> I'm not sure I understand this bug. content usually isn't null for about:blank,
> is it?
It is ;-)

What's actually happening here is that we're opening a bunch of windows asynchronously, and we don't want windows that open after the window to focus to steal focus from the window to focus. Unfortunately the code is also used for windows that open before the window to focus, and the window to focus hasn't loaded its chrome yet and therefore has no content.

Comment 12

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I forgot to comment 2 months ago, but this was pushed to branch as http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8aafaae15cce