Closed Bug 746837 Opened 9 years ago Closed 9 years ago

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


(Firefox :: Session Restore, defect)

Not set



Firefox 14


(Reporter: khuey, Assigned: dao)




(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
This will throw after Bug 695480 lands.
Attachment #616385 - Flags: review?(paul)
This should probably just be calling browser.focus()?
Maybe!  I don't have any idea what I'm really doing here ...
(In reply to :Gavin Sharp (use 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.
Attached patch better patch?Splinter Review
It seems that we don't even need the setTimeout anymore. This doesn't regress bug 342432 comment 6.
Attachment #616488 - Flags: review?(paul)
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.
Attachment #616488 - Flags: review?(paul) → review+
Assignee: khuey → dao
Attachment #616385 - Attachment is obsolete: true
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
(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).
You need to log in before you can comment on or make changes to this bug.