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

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Session Restore
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: khuey, Assigned: dao)

Tracking

unspecified
Firefox 14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 616385 [details] [diff] [review]
Patch

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 ...
(Assignee)

Comment 3

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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.
(Assignee)

Comment 4

5 years ago
Created attachment 616488 [details] [diff] [review]
better patch?

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+
Attachment #616385 - Flags: review?(paul)
(Assignee)

Updated

5 years ago
Assignee: khuey → dao
(Assignee)

Updated

5 years ago
Attachment #616385 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
http://hg.mozilla.org/mozilla-central/rev/d1ac8e24872c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
Blocks: 749106

Comment 7

5 years ago
(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.