Closed Bug 642998 Opened 11 years ago Closed 11 years ago

Support tabbrowser API removeTab extension

Categories

(SeaMonkey :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(1 file, 1 obsolete file)

Firefox's tabbed browser extends removeTab by supporting a parameter object. Currently SeaMonkey uses a boolean flag to disable its undo. We should switch this to a parameter object which will also help us support the animate flag.
Attached patch Proposed patch (obsolete) — Splinter Review
I noticed that we don't pass the don't undo flag to session store. This means that we can undo close tab after print preview when we shouldn't be able to.

I wasn't sure what the test was trying to do though, so that might be wrong.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #520333 - Flags: review?(misak.bugzilla)
Comment on attachment 520333 [details] [diff] [review]
Proposed patch

The test is wrong, because on the moment of writing after hours of debugging a was stupid enough to not see that i'm clearing browser.tabs.max_tabs_undo pref on the wrong place, so i added don't undo param to be sure that sessionstore will restore tab, not our tabbrowser. So we don't need disableUndo here. I don't know the right procedure how to deal with this situation, so r+ if you review (and r+) and add this diff ;)

diff --git a/suite/common/tests/browser/browser_615394-SSWindowState_events.js b/suite/common/tests/browser/browser_615394-SSWindowState_events.js
--- a/suite/common/tests/browser/browser_615394-SSWindowState_events.js
+++ b/suite/common/tests/browser/browser_615394-SSWindowState_events.js
@@ -77,17 +77,16 @@ function getOuterWindowID(aWindow) {
 
 function test() {
   /** Test for Bug 615394 - Session Restore should notify when it is beginning and ending a restore **/
   waitForExplicitFinish();
   // Preemptively extend the timeout to prevent [orange]
   requestLongerTimeout(2);
   Services.prefs.setIntPref("browser.tabs.max_tabs_undo", 0);
   runNextTest();
-  Services.prefs.clearUserPref("browser.tabs.max_tabs_undo");
 }
 
 
 let tests = [
   test_setTabState,
   test_duplicateTab,
   test_undoCloseTab,
   test_setWindowState,
@@ -107,16 +106,17 @@ function runNextTest() {
       }
     }
 
     let currentTest = tests.shift();
     info("prepping for " + currentTest.name);
     waitForBrowserState(testState, currentTest);
   }
   else {
+    Services.prefs.clearUserPref("browser.tabs.max_tabs_undo");
     ss.setBrowserState(stateBackup);
     finish();
   }
 }
 
 /**  ACTUAL TESTS  **/
 
 function test_setTabState() {
@@ -202,24 +202,22 @@ function test_undoCloseTab() {
       busyEventCount = 0,
       readyEventCount = 0,
       reopenedTab;
 
   ss.setTabValue(tab, "foo", "bar");
 
   function onSSWindowStateBusy(aEvent) {
     busyEventCount++;
-    dump("onSSWindowStateBusy\n")
   }
 
   // undoCloseTab is "synchronous" in tab creation. Since restoreHistory is called
   // via setTimeout, reopenedTab will be assigned before the SSWindowStateReady event
   function onSSWindowStateReady(aEvent) {
     readyEventCount++;
-    dump("onSSWindowStateReady");
     is(ss.getTabValue(reopenedTab, "foo"), "bar");
     ss.setTabValue(reopenedTab, "baz", "qux");
   }
 
   function onSSTabRestored(aEvent) {
     is(busyEventCount, 1);
     is(readyEventCount, 1);
     is(ss.getTabValue(reopenedTab, "baz"), "qux");
@@ -231,17 +229,17 @@ function test_undoCloseTab() {
 
     runNextTest();
   }
 
   window.addEventListener("SSWindowStateBusy", onSSWindowStateBusy, false);
   window.addEventListener("SSWindowStateReady", onSSWindowStateReady, false);
   getBrowser().tabContainer.addEventListener("SSTabRestored", onSSTabRestored, false);
 
-  getBrowser().removeTab(tab, true);
+  getBrowser().removeTab(tab);
   reopenedTab = ss.undoCloseTab(window, 0);
 }
 
 
 function test_setWindowState() {
   let testState = {
     windows: [{
       tabs: [
Attachment #520333 - Flags: review?(misak.bugzilla) → review+
Attached patch Fixed patchSplinter Review
* Used the fixed test (test passes locally)
* Fixed two tests for blank tabs
  (one to stop them going into undo cache, one is for when you close the last
   tab using the close button; this replaces the tab with a blank tab, and
   undoing that should delete the blank tab.)
Attachment #520333 - Attachment is obsolete: true
Attachment #520476 - Flags: review?(misak.bugzilla)
Blocks: 639836
Attachment #520476 - Flags: review?(misak.bugzilla) → review+
Pushed: http://hg.mozilla.org/comm-central/rev/f1e8aa0e8de3
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.