639852 - Port Bug 627642 [Don't restore session into a new window if only the home page is open]

Status: RESOLVED FIXED

(SeaMonkey :: Session Restore, defect)

Description

[Misak Khachatryan]

Attachment: patch

From parent bug:

The original work in bug 588482 is pretty good, but it was pretty dumb in
determining if a window should be reused.

The most common case now is going to be a single tab (about:home). We can check
for a single about:home tab & no extData (which makes the panorama case
harder), no recently closed tabs, no history in that single tab. We could
probably ignore the extData case (potential for data loss but small) so that
entering panorama and pressing restore previous session doesn't suck.

With bug 592822 landing and soon bug 593421 and bug 589665, I think this is
going to be a much bigger deal than it was before.

Comment 1

[[e-mail]]

Comment on attachment 517756 [details] [diff] [review]
patch

>+        tabbrowser.removeTab(removableTabs.pop(), { animate: false });
On an unrelated note, tabbrowser.removeTab actually already has a second parameter, aDisableUndo. I wonder how many problems that causes?

Comment 2

[[e-mail]]

Comment on attachment 517756 [details] [diff] [review]
patch

>+    let homePages = aWindow.gHomeButton.getHomePage().split("|");
This is just aWindow.getHomePage() in our case.

Comment 3

[[e-mail]]

(In reply to comment #1)
>(From update of attachment 517756 [details] [diff] [review])
>>+        tabbrowser.removeTab(removableTabs.pop(), { animate: false });
>On an unrelated note, tabbrowser.removeTab actually already has a second
>parameter, aDisableUndo. I wonder how many problems that causes?
It looks like browser_bug617076.js also calls removeTab with animate set to false. I guess I need to file a bug on supporting parmeters to removeTab.

Comment 4

[Misak Khachatryan]

Attachment: updated patch with nit

Comment 5

[[e-mail]]

Comment on attachment 521479 [details] [diff] [review]
updated patch with nit

>+    // Step 2 of processing:
Aside: This really belongs in a separate method. Then the caller can be
>// If there's a window already open that we can restore into, use that
>if (this._canUseWindow(windowToUse) {
>  let canOverwriteTabs = this._prepWindowToRestoreInto(windowToUse);

>+    let normalTabsLen = tabbrowser.tabs.length - tabbrowser._numPinnedTabs;
>+    for (let i = tabbrowser._numPinnedTabs; i < tabbrowser.tabs.length; i++) {
We don't have pinned tabs so this isn't going to work. I tried changing this to
>for (let i = 0; i < tabbrowser.tabs.length; i++) {
and this seems to make it work. If this is the correct fix, r=me.

Comment 6

[Misak Khachatryan]

Attachment: what actually checked in

Pushed: http://hg.mozilla.org/comm-central/rev/1c816798b5df