746837 - Fix sessionstore to handle an exception thrown when attempting to focus a window that has been navigated

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This will throw after Bug 695480 lands.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This should probably just be calling browser.focus()?

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Maybe!  I don't have any idea what I'm really doing here ...

Comment 3

[Dão Gottwald [:dao]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #1)
> This should probably just be calling browser.focus()?

'browser' is the tabbrowser here. It should be renamed accordingly, 'let browser = tabbrowser.getBrowserForTab(tab);' should be added and then we can focus the browser in the callback function.

Comment 4

[Dão Gottwald [:dao]]

Attachment: better patch?

It seems that we don't even need the setTimeout anymore. This doesn't regress bug 342432 comment 6.

Comment 5

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

Comment on attachment 616488 [details] [diff] [review]
better patch?

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

::: browser/components/sessionstore/src/nsSessionStore.js
@@ +1475,3 @@
>  
>      // focus the tab's content area
> +    tabbrowser.getBrowserForTab(tab).focus();

Make this `tab.linkedBrowser` and we have a deal.

Comment 6

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/d1ac8e24872c

Comment 7

[[e-mail]]

(In reply to Gavin Sharp from comment #1)
> This should probably just be calling browser.focus()?

Indeed, as of the great focus rewrite (somewhere between 3.5 and 4, I forget when) focusing the browser is the way to go (except in certain unrelated cases where you specifically want to raise the window at the same time).