Last Comment Bug 524345 - Port Bug 461634 [new API: allow to delete a single closed tab] to SeaMonkey
: Port Bug 461634 [new API: allow to delete a single closed tab] to SeaMonkey
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
-- normal (vote)
: seamonkey2.1a1
Assigned To: Misak Khachatryan
:
:
Mentors:
Depends on: 461634 478707
Blocks: 567531
  Show dependency treegraph
 
Reported: 2009-10-25 04:44 PDT by Misak Khachatryan
Modified: 2010-05-22 05:09 PDT (History)
0 users
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (7.89 KB, patch)
2009-10-30 04:08 PDT, Misak Khachatryan
neil: superreview-
Details | Diff | Splinter Review
suggestion implemented. (9.59 KB, patch)
2009-11-02 04:54 PST, Misak Khachatryan
no flags Details | Diff | Splinter Review
comments fixed. (10.02 KB, patch)
2009-11-03 02:58 PST, Misak Khachatryan
neil: review+
neil: superreview+
Details | Diff | Splinter Review
for checkin [Checkin: Comment 10] (10.03 KB, patch)
2009-11-05 21:31 PST, Misak Khachatryan
misak.bugzilla: review+
misak.bugzilla: superreview+
Details | Diff | Splinter Review

Description User image Misak Khachatryan 2009-10-25 04:44:19 PDT
From parent bug:

Extensions desiring to selectively forget information about closed tabs/windows
have to be quite inventive. What about a new API:

void forgetClosedTab(in nsIDOMWindow aWindow, in unsigned long aIndex);
Comment 1 User image Misak Khachatryan 2009-10-30 04:08:03 PDT
Created attachment 409308 [details] [diff] [review]
patch

Patch with test. I disabled some of tests related to our specific implementation of undCloseTab and bug 478707. Rest of tests passing.
Comment 2 User image neil@parkwaycc.co.uk 2009-10-30 05:19:07 PDT
Comment on attachment 409308 [details] [diff] [review]
patch

I don't think this relates to our version of undoCloseTab.
Comment 3 User image Misak Khachatryan 2009-10-30 05:26:42 PDT
Sorry, i just mean that because of our specific implementation of undoCloseTab, bug 478707 exist, and we have little mess with getClosedTabData. We don't restore closed tab data or save it when setWindowState called. That's why disabled test fail.
Comment 4 User image neil@parkwaycc.co.uk 2009-10-30 05:46:22 PDT
Right, but this API is to forget about the closed tab of an open window, and we currently track that on the tabbrowser instead. At least from our point of view, it would make more sense to provide the API on the tabbrowser too, and it would simply destroy the saved browser in much the same way as the code at the end of removeTab destroys the oldest saved browser if it overflows the undo stack.
Comment 5 User image Misak Khachatryan 2009-11-02 04:54:03 PST
Created attachment 409676 [details] [diff] [review]
suggestion implemented.

Done as Neil suggested. Also i replaced gBrowser by getBrowser(). Test is almost useless now, it only check for wrong parameters, until bug 478707 will be fixed.
Comment 6 User image neil@parkwaycc.co.uk 2009-11-02 05:48:07 PST
Comment on attachment 409676 [details] [diff] [review]
suggestion implemented.

>+      <method name="forgetSavedBrowser">
I wonder whether Firefox would be interested in an API on tabbrowser.

>+        <parameter name="aIndex"/>
Need to validate aIndex here, in case anyone calls it directly.

>+    aIndex = aIndex || 0;
Don't need this, XPConnect already defaults to 0. (And you'll be validating in the tabbrowser anyway, right?)

>+    ss.setWindowState(newWin, JSON.stringify(test_state), true);
Maybe we can add and close tabs directly, thus simulating closed tabs?
Comment 7 User image Misak Khachatryan 2009-11-03 02:58:36 PST
Created attachment 409893 [details] [diff] [review]
comments fixed.

Well, all done as suggested. Tests all enabled and passing.
Comment 8 User image neil@parkwaycc.co.uk 2009-11-05 05:22:16 PST
Comment on attachment 409893 [details] [diff] [review]
comments fixed.

Warning: 9 lines add whitespace errors.

>diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js
>--- a/suite/common/src/nsSessionStore.js
>+++ b/suite/common/src/nsSessionStore.js
>@@ -23,6 +23,7 @@
>  *   Ehsan Akhgari <ehsan.akhgari@gmail.com>
>  *   Paul O’Shannessy <paul@oshannessy.com>
>  *   Nils Maier <maierman@web.de>
>+ *   Michael Kraft <morac99-firefox@yahoo.com>
In this file, the changes are all yours :-)

>+	"... and tabs not specifically forgetten weren't.");
Nit: forgotten
Comment 9 User image Misak Khachatryan 2009-11-05 21:31:22 PST
Created attachment 410720 [details] [diff] [review]
for checkin
[Checkin: Comment 10]

patch for checkin, carrying forward r+ and sr+ from Neil.
Comment 10 User image Serge Gautherie (:sgautherie) 2009-11-09 19:33:53 PST
Comment on attachment 410720 [details] [diff] [review]
for checkin
[Checkin: Comment 10]


http://hg.mozilla.org/comm-central/rev/401efe3c7c6a
Comment 11 User image Misak Khachatryan 2009-11-11 22:16:09 PST
I guess we can close this bug.

Note You need to log in before you can comment on or make changes to this bug.