Bug 629195 - Restore previous session does bad things if you have an 'undo close group' and an app tab [r=ian, a=sdwilsh]
authorRaymond Lee <raymond@raysquare.com>
Thu, 03 Feb 2011 23:40:54 +0800
changeset 61856 602d24b981e53778e7dc5fcc0fee212f6c8af716
parent 61855 562ae97b263612a46f215d9212d8377acf9cc378
child 61857 490e39e1d9569f606e3ff5675c9569b4c7b8529d
push idunknown
push userunknown
push dateunknown
reviewersian, sdwilsh
bugs629195
milestone2.0b12pre
Bug 629195 - Restore previous session does bad things if you have an 'undo close group' and an app tab [r=ian, a=sdwilsh]
browser/base/content/tabview/groupitems.js
browser/base/content/test/tabview/Makefile.in
browser/base/content/test/tabview/browser_tabview_bug629195.js
--- a/browser/base/content/tabview/groupitems.js
+++ b/browser/base/content/tabview/groupitems.js
@@ -668,16 +668,32 @@ GroupItem.prototype = Utils.extend(new I
         !remainingGroups.length) {
       let emptyGroups = GroupItems.groupItems.filter(function (groupItem) {
         return (groupItem != self && !groupItem.getChildren().length);
       });
       let group = (emptyGroups.length ? emptyGroups[0] : GroupItems.newGroup());
       group.newTab();
     }
 
+    this.destroy();
+  },
+
+  // ----------
+  // Function: destroy
+  // Close all tabs linked to children (tabItems), removes all children and 
+  // close the groupItem.
+  //
+  // Parameters:
+  //   options - An object with optional settings for this call.
+  //
+  // Options:
+  //   immediately - (bool) if true, no animation will be used
+  destroy: function GroupItem_destroy(options) {
+    let self = this;
+
     // when "TabClose" event is fired, the browser tab is about to close and our 
     // item "close" event is fired.  And then, the browser tab gets closed. 
     // In other words, the group "close" event is fired before all browser
     // tabs in the group are closed.  The below code would fire the group "close"
     // event only after all browser tabs in that group are closed.
     let shouldRemoveTabItems = [];
     let toClose = this._children.concat();
     toClose.forEach(function(child) {
@@ -698,17 +714,17 @@ GroupItem.prototype = Utils.extend(new I
     if (shouldRemoveTabItems.length != toClose.length) {
       // remove children without the assiciated tab and show the group item
       shouldRemoveTabItems.forEach(function(child) {
         self.remove(child, { dontArrange: true });
       });
 
       this.$undoContainer.fadeOut(function() { self._unhide() });
     } else {
-      this.close();
+      this.close(options);
     }
   },
 
   // ----------
   // Function: _fadeAwayUndoButton
   // Fades away the undo button
   _fadeAwayUndoButton: function GroupItem__fadeAwayUdoButton() {
     let self = this;
@@ -1887,17 +1903,16 @@ let GroupItems = {
     var bottom = Math.max.apply({},[ b.bottom for each (b in bounds) ]);
 
     return new Rect(left, top, right-left, bottom-top);
   },
 
   // ----------
   // Function: reconstitute
   // Restores to stored state, creating groupItems as needed.
-  // If no data, sets up blank slate (including "new tabs" groupItem).
   reconstitute: function GroupItems_reconstitute(groupItemsData, groupItemData) {
     try {
       let activeGroupId;
 
       if (groupItemsData) {
         if (groupItemsData.nextID)
           this.nextID = Math.max(this.nextID, groupItemsData.nextID);
         if (groupItemsData.activeGroupId)
@@ -1905,17 +1920,17 @@ let GroupItems = {
       }
 
       if (groupItemData) {
         var toClose = this.groupItems.concat();
         for (var id in groupItemData) {
           let data = groupItemData[id];
           if (this.groupItemStorageSanity(data)) {
             let groupItem = this.groupItem(data.id); 
-            if (groupItem) {
+            if (groupItem && !groupItem.hidden) {
               groupItem.userSize = data.userSize;
               groupItem.setTitle(data.title);
               groupItem.setBounds(data.bounds, true);
               
               let index = toClose.indexOf(groupItem);
               if (index != -1)
                 toClose.splice(index, 1);
             } else {
@@ -1925,17 +1940,17 @@ let GroupItems = {
               };
   
               new GroupItem([], Utils.extend({}, data, options));
             }
           }
         }
 
         toClose.forEach(function(groupItem) {
-          groupItem.close({immediately: true});
+          groupItem.destroy({immediately: true});
         });
       }
 
       // set active group item
       if (activeGroupId) {
         let activeGroupItem = this.groupItem(activeGroupId);
         if (activeGroupItem)
           this.setActiveGroupItem(activeGroupItem);
--- a/browser/base/content/test/tabview/Makefile.in
+++ b/browser/base/content/test/tabview/Makefile.in
@@ -94,16 +94,17 @@ include $(topsrcdir)/config/rules.mk
                  browser_tabview_bug624953.js \
                  browser_tabview_bug625269.js \
                  browser_tabview_bug625424.js \
                  browser_tabview_bug626368.js \
                  browser_tabview_bug627288.js \
                  browser_tabview_bug627736.js \
                  browser_tabview_bug628165.js \
                  browser_tabview_bug628270.js \
+                 browser_tabview_bug629195.js \
                  browser_tabview_dragdrop.js \
                  browser_tabview_exit_button.js \
                  browser_tabview_expander.js \
                  browser_tabview_group.js \
                  browser_tabview_launch.js \
                  browser_tabview_multiwindow_search.js \
                  browser_tabview_orphaned_tabs.js \
                  browser_tabview_privatebrowsing.js \
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabview/browser_tabview_bug629195.js
@@ -0,0 +1,126 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let win;
+let contentWindow;
+let originalTab;
+
+function test() {
+  waitForExplicitFinish();
+  setup();
+}
+
+function setup() {
+  win = window.openDialog(getBrowserURL(), "_blank", 
+                          "chrome,all,dialog=no,height=800,width=800");
+
+  let onLoad = function() {
+    win.removeEventListener("load", onLoad, false);
+
+    originalTab = win.gBrowser.visibleTabs[0];
+    win.gBrowser.addTab();
+    win.gBrowser.pinTab(originalTab);
+
+    let onTabViewShown = function() {
+      win.removeEventListener("tabviewshown", onTabViewShown, false);
+
+      ok(win.TabView.isVisible(), "Tab View is visible");
+
+      contentWindow = win.document.getElementById("tab-view").contentWindow;
+      is(contentWindow.GroupItems.groupItems.length, 1, "There is one group");
+      is(contentWindow.GroupItems.groupItems[0].getChildren().length, 1, 
+         "The group has only one tab item");
+
+      // show the undo close group button
+      let group = contentWindow.GroupItems.groupItems[0];
+      group.closeAll();
+
+      group.addSubscriber(group, "groupHidden", function() {
+        group.removeSubscriber(group, "groupHidden");
+        restore(group.id);
+      });
+
+    };
+    win.addEventListener("tabviewshown", onTabViewShown, false);
+    win.TabView.toggle();
+  }
+  win.addEventListener("load", onLoad, false);
+}
+
+function restore(groupId) {
+  // window state ready
+  let handleSSWindowStateReady = function() {
+    win.removeEventListener("SSWindowStateReady", handleSSWindowStateReady, false);
+
+    executeSoon(function() { 
+      is(contentWindow.GroupItems.groupItems.length, 1, "There is one group");
+
+      let group = contentWindow.GroupItems.groupItems[0];
+      ok(!group.hidden, "The group is visible");
+      is(group.getChildren().length, 2, "This group has two tab items");
+
+      // check the position of the group item and the tab items.
+      let tabItemOne = group.getChildren()[0];
+      let tabItemTwo = group.getChildren()[1];
+
+      let groupBounds = group.getBounds();
+      let tabItemOneBounds = tabItemOne.getBounds();
+      let tabItemTwoBounds = tabItemTwo.getBounds();
+
+      ok(groupBounds.left < tabItemOneBounds.left && 
+         (groupBounds.right) > (tabItemOneBounds.right) &&
+         groupBounds.top < tabItemOneBounds.top && 
+         (groupBounds.bottom) > (tabItemOneBounds.bottom), 
+         "Tab item one is within the group");
+
+      ok(groupBounds.left < tabItemOneBounds.left && 
+         (groupBounds.right) > (tabItemTwoBounds.right) &&
+         groupBounds.top < tabItemOneBounds.top && 
+         (groupBounds.bottom) > (tabItemTwoBounds.bottom), 
+         "Tab item two is within the group");
+
+      win.close();
+      finish();
+    });
+  }
+  win.addEventListener("SSWindowStateReady", handleSSWindowStateReady, false);
+
+  // simulate restoring previous session (one group and two tab items)
+  const DUMMY_PAGE_URL = "http://example.com/";
+  let newState = {
+    windows: [{
+      tabs: [{
+        entries: [{ url: DUMMY_PAGE_URL }],
+        index: 2,
+        hidden: false,
+        attributes: {},
+        extData: {
+          "tabview-tab":
+            '{"bounds":{"left":208,"top":54,"width":205,"height":169},' +
+            '"userSize":null,"url":"' + DUMMY_PAGE_URL + '","groupID":' + 
+            groupId + ',"imageData":null,"title":null}'
+        }}, {
+        entries: [{ url: DUMMY_PAGE_URL }],
+        index: 1,
+        hidden: false,
+        attributes: {},
+        extData: {
+          "tabview-tab":
+            '{"bounds":{"left":429,"top":54,"width":205,"height":169},' +
+            '"userSize":null,"url":"' + DUMMY_PAGE_URL + '","groupID":' + 
+            groupId + ',"imageData":null,"title":null}'
+        }
+      }],
+      extData: {
+        "tabview-groups": '{"nextID":' + (groupId + 1) + ',"activeGroupId":' + groupId + '}',
+        "tabview-group": 
+          '{"' + groupId + '":{"bounds":{"left":202,"top":30,"width":455,"height":249},' + 
+          '"userSize":null,"locked":{},"title":"","id":' + groupId +'}}',
+        "tabview-ui": '{"pageBounds":{"left":0,"top":0,"width":788,"height":548}}'
+      }, sizemode:"normal"
+    }]
+  };
+  let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
+  ss.setWindowState(win, JSON.stringify(newState), false);
+}
+