Bug 634672 - Minimum tab size is no longer being respected [r=ian]

browser/base/content/tabview/groupitems.js

@@ -1237,8 +1237,8 @@ GroupItem.prototype = Utils.extend(new Item(), new Subscribable(), {
       count: count || this._children.length,
       hideTitle: false
     };
-    let arrObj = Items.arrange(null, bb, options);
- 
+    let arrObj = Items.arrange(this._children, bb, options);
+
     let shouldStack = arrObj.childWidth < TabItems.minTabWidth * 1.35;
     this._columns = shouldStack ? null : arrObj.columns;
 

browser/base/content/tabview/items.js

@@ -914,7 +914,6 @@ let Items = {
   //     width of children and the columns.
   //   count - overrides the item count for layout purposes;
   //     default: the actual item count
-  //   padding - pixels between each item
   //   columns - (int) a preset number of columns to use
   //   dropPos - a <Point> which should have a one-tab space left open, used
   //             when a tab is dragged over.

browser/base/content/tabview/tabitems.js

@@ -229,16 +229,6 @@ TabItem.prototype = Utils.extend(new Item(), new Subscribable(), {
     this.tabCanvas.paint();
   },
 
-  // ----------
-  // Function: _getFontSizeFromWidth
-  // Private method that returns the fontsize to use given the tab's width
-  _getFontSizeFromWidth: function TabItem__getFontSizeFromWidth(width) {
-    let widthRange = new Range(0,TabItems.tabWidth);
-    let proportion = widthRange.proportion(width-TabItems.tabItemPadding.x, true);
-    // proportion is in [0,1]
-    return TabItems.fontSizeRange.scale(proportion);
-  },
-
   // ----------
   // Function: unforceCanvasSize
   // Stops holding the thumbnail resolution; allows it to shift to the
@@ -453,16 +443,14 @@ TabItem.prototype = Utils.extend(new Item(), new Subscribable(), {
 
     if (rect.width != this.bounds.width || options.force) {
       css.width = rect.width - TabItems.tabItemPadding.x;
-      css.fontSize = this._getFontSizeFromWidth(rect.width);
+      css.fontSize = TabItems.getFontSizeFromWidth(rect.width);
       css.fontSize += 'px';
     }
 
     if (rect.height != this.bounds.height || options.force) {
+      css.height = rect.height - TabItems.tabItemPadding.y;
       if (!this.isStacked)
-          css.height = rect.height - TabItems.tabItemPadding.y -
-                       TabItems.fontSizeRange.max;
-      else
-        css.height = rect.height - TabItems.tabItemPadding.y;
+        css.height -= TabItems.fontSizeRange.max;
     }
 
     if (Utils.isEmptyObject(css))
@@ -1293,57 +1281,59 @@ let TabItems = {
 
     return sane;
   },
-  
+
+  // ----------
+  // Function: getFontSizeFromWidth
+  // Private method that returns the fontsize to use given the tab's width
+  getFontSizeFromWidth: function TabItem_getFontSizeFromWidth(width) {
+    let widthRange = new Range(0, TabItems.tabWidth);
+    let proportion = widthRange.proportion(width - TabItems.tabItemPadding.x, true);
+    // proportion is in [0,1]
+    return TabItems.fontSizeRange.scale(proportion);
+  },
+
   // ----------
   // Function: _getWidthForHeight
   // Private method that returns the tabitem width given a height.
-  // Set options.hideTitle=true to measure without a title.
-  // Default is to measure with a title.
-  _getWidthForHeight: function TabItems__getWidthForHeight(height, options) {    
-    let titleSize = (options !== undefined && options.hideTitle === true) ? 
-      0 : TabItems.fontSizeRange.max;
-    return Math.max(0, Math.max(TabItems.minTabHeight, height - titleSize)) * 
-      TabItems.invTabAspect;
+  _getWidthForHeight: function TabItems__getWidthForHeight(height) {
+    return height * TabItems.invTabAspect;
   },
 
   // ----------
   // Function: _getHeightForWidth
   // Private method that returns the tabitem height given a width.
-  // Set options.hideTitle=false to measure without a title.
-  // Default is to measure with a title.
-  _getHeightForWidth: function TabItems__getHeightForWidth(width, options) {
-    let titleSize = (options !== undefined && options.hideTitle === true) ? 
-      0 : TabItems.fontSizeRange.max;
-    return Math.max(0, Math.max(TabItems.minTabWidth,width)) *
-      TabItems.tabAspect + titleSize;
+  _getHeightForWidth: function TabItems__getHeightForWidth(width) {
+    return width * TabItems.tabAspect;
   },
-  
+
   // ----------
   // Function: calcValidSize
   // Pass in a desired size, and receive a size based on proper title
   // size and aspect ratio.
   calcValidSize: function TabItems_calcValidSize(size, options) {
     Utils.assert(Utils.isPoint(size), 'input is a Point');
-    let retSize = new Point(0,0);
-    if (size.x==-1) {
-      retSize.x = this._getWidthForHeight(size.y, options);
-      retSize.y = size.y;
-    } else if (size.y==-1) {
-      retSize.x = size.x;
-      retSize.y = this._getHeightForWidth(size.x, options);
-    } else {
-      let fitHeight = this._getHeightForWidth(size.x, options);
-      let fitWidth = this._getWidthForHeight(size.y, options);
 
-      // Go with the smallest final dimension.
-      if (fitWidth < size.x) {
-        retSize.x = fitWidth;
-        retSize.y = size.y;
-      } else {
-        retSize.x = size.x;
-        retSize.y = fitHeight;
-      }
+    let width = Math.max(TabItems.minTabWidth, size.x);
+    let showTitle = !options || !options.hideTitle;
+    let titleSize = showTitle ? TabItems.fontSizeRange.max : 0;
+    let height = Math.max(TabItems.minTabHeight, size.y - titleSize);
+    let retSize = new Point(width, height);
+
+    if (size.x > -1)
+      retSize.y = this._getHeightForWidth(width);
+    if (size.y > -1)
+      retSize.x = this._getWidthForHeight(height);
+
+    if (size.x > -1 && size.y > -1) {
+      if (retSize.x < size.x)
+        retSize.y = this._getHeightForWidth(retSize.x);
+      else
+        retSize.x = this._getWidthForHeight(retSize.y);
     }
+
+    if (showTitle)
+      retSize.y += titleSize;
+
     return retSize;
   }
 };

browser/base/content/test/tabview/Makefile.in

@@ -105,6 +105,7 @@ _BROWSER_FILES = \
                  browser_tabview_bug624953.js \
                  browser_tabview_bug625269.js \
                  browser_tabview_bug625424.js \
+                 browser_tabview_bug625666.js \
                  browser_tabview_bug626368.js \
                  browser_tabview_bug626525.js \
                  browser_tabview_bug626791.js \
@@ -122,6 +123,7 @@ _BROWSER_FILES = \
                  browser_tabview_bug634077.js \
                  browser_tabview_bug634085.js \
                  browser_tabview_bug634158.js \
+                 browser_tabview_bug634672.js \
                  browser_tabview_bug635696.js \
                  browser_tabview_bug641802.js \
                  browser_tabview_bug645653.js \

browser/base/content/test/tabview/browser_tabview_bug610208.js

@@ -125,18 +125,20 @@ function test() {
   // make sure we don't freeze item size when removing an item from a stack
   let testRemoveWhileStacked = function () {
     let oldBounds = groupItem.getBounds();
-    groupItem.setSize(150, 200, true);
+    groupItem.setSize(250, 250, true);
     groupItem.setUserSize();
 
     let originalBounds = groupItem.getChild(0).getBounds();
-    ok(!groupItem._isStacked, 'testRemoveWhileStacked: group is not stacked');
+    ok(!groupItem.isStacked(), 'testRemoveWhileStacked: group is not stacked');
 
-    // add a new tab to let the group stack
-    win.gBrowser.loadOneTab('about:blank', {inBackground: true});
-    ok(groupItem._isStacked, 'testRemoveWhileStacked: group is now stacked');
+    // add new tabs to let the group stack
+    while (!groupItem.isStacked())
+      win.gBrowser.loadOneTab('about:blank', {inBackground: true});
 
     afterAllTabsLoaded(function () {
       groupItem.getChild(0).close();
+      ok(!groupItem.isStacked(), 'testRemoveWhileStacked: group is not stacked');
+
       let bounds = groupItem.getChild(0).getBounds();
       ok(originalBounds.equals(bounds), 'testRemoveWhileStacked: tabs did not change their size');
 
@@ -156,7 +158,7 @@ function test() {
     groupItem.setSize(100, 100, true);
     groupItem.setUserSize();
 
-    ok(groupItem._isStacked, 'testExpandedMode: group is stacked');
+    ok(groupItem.isStacked(), 'testExpandedMode: group is stacked');
 
     groupItem.addSubscriber(groupItem, 'expanded', function () {
       groupItem.removeSubscriber(groupItem, 'expanded');
@@ -173,7 +175,7 @@ function test() {
       let tabItem = groupItem.getChild(1);
       let bounds = tabItem.getBounds();
 
-      for (let i=0; i<3; i++)
+      while (groupItem.getChildren().length > 2)
         groupItem.getChild(1).close();
 
       ok(originalBounds.equals(groupItem.getChild(0).getBounds()), 'testExpandedMode: tabs did not change their size');

browser/base/content/test/tabview/browser_tabview_bug624953.js

@@ -4,36 +4,20 @@
 function test() {
   waitForExplicitFinish();
 
-  let finishTest = function (groupItem) {
-    groupItem.addSubscriber(groupItem, 'groupHidden', function () {
-      groupItem.removeSubscriber(groupItem, 'groupHidden');
-      groupItem.closeHidden();
-      hideTabView(finish);
-    });
-
-    groupItem.closeAll();
-  }
-
   showTabView(function () {
     let cw = TabView.getContentWindow();
 
-    let bounds = new cw.Rect(20, 20, 150, 200);
-    let groupItem = new cw.GroupItem([], {bounds: bounds, immediately: true});
-    cw.GroupItems.setActiveGroupItem(groupItem);
-
-    for (let i=0; i<4; i++)
-      gBrowser.loadOneTab('about:blank', {inBackground: true});
-
-    ok(!groupItem._isStacked, 'groupItem is not stacked');
+    let groupItem = createGroupItemWithBlankTabs(window, 200, 240, 20, 4)
+    ok(!groupItem.isStacked(), 'groupItem is not stacked');
     cw.GroupItems.pauseArrange();
 
-    groupItem.setSize(150, 150);
+    groupItem.setSize(100, 100, true);
     groupItem.setUserSize();
-    ok(!groupItem._isStacked, 'groupItem is still not stacked');
+    ok(!groupItem.isStacked(), 'groupItem is still not stacked');
 
     cw.GroupItems.resumeArrange();
-    ok(groupItem._isStacked, 'groupItem is now stacked');
+    ok(groupItem.isStacked(), 'groupItem is now stacked');
 
-    finishTest(groupItem);
+    closeGroupItem(groupItem, finish);
   });
 }

browser/base/content/test/tabview/browser_tabview_bug625666.js

@@ -0,0 +1,49 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function test() {
+  let cw;
+
+  let getTabItemAspect = function (tabItem) {
+    let bounds = cw.iQ('.thumb', tabItem.container).bounds();
+    let padding = cw.TabItems.tabItemPadding;
+    return (bounds.height + padding.y) / (bounds.width + padding.x);
+  }
+
+  let getAspectRange = function () {
+    let aspect = cw.TabItems.tabAspect;
+    let variance = aspect / 100 * 1.5;
+    return new cw.Range(aspect - variance, aspect + variance);
+  }
+
+  waitForExplicitFinish();
+
+  newWindowWithTabView(function (win) {
+    registerCleanupFunction(function () win.close());
+    cw = win.TabView.getContentWindow();
+
+    // prepare orphan tab
+    let tabItem = win.gBrowser.tabs[0]._tabViewTabItem;
+    tabItem.parent.remove(tabItem, {immediately: true});
+    tabItem.setBounds(new cw.Rect(20, 20, 200, 165), true);
+
+    let bounds = tabItem.getBounds();
+
+    // prepare group item
+    let box = new cw.Rect(20, 300, 400, 200);
+    let groupItem = new cw.GroupItem([], {bounds: box, immediately: true});
+
+    groupItem.setBounds(new cw.Rect(20, 100, 400, 200));
+    groupItem.pushAway(true);
+
+    let newBounds = tabItem.getBounds();
+    ok(newBounds.width < bounds.width, "The new width of item is smaller than the old one.");
+    ok(newBounds.height < bounds.height, "The new height of item is smaller than the old one.");
+
+    let aspectRange = getAspectRange();
+    let aspect = getTabItemAspect(tabItem);
+    ok(aspectRange.contains(aspect), "orphaned tabItem's aspect is correct");
+
+    finish();
+  });
+}

browser/base/content/test/tabview/browser_tabview_bug634672.js

@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function test() {
+  let cw;
+  let win;
+
+  waitForExplicitFinish();
+
+  newWindowWithTabView(function (tvwin) {
+    win = tvwin;
+    cw = win.TabView.getContentWindow();
+
+    registerCleanupFunction(function () {
+      if (win && !win.closed)
+        win.close();
+    });
+
+    // fill the group item with some tabs
+    for (let i = 0; i < 5; i++)
+      win.gBrowser.loadOneTab("about:blank");
+
+    let groupItem = cw.GroupItems.groupItems[0];
+    groupItem.setSize(400, 400, true);
+    let range = new cw.Range(1, 400);
+
+    // determine the groupItem's largest possible stacked size
+    while (range.extent > 1) {
+      let pivot = Math.floor(range.extent / 2);
+      groupItem.setSize(range.min + pivot, range.min + pivot, true);
+
+      if (groupItem.isStacked())
+        range.min += pivot;
+      else
+        range.max -= pivot;
+    }
+
+    // stack the group
+    groupItem.setSize(range.min, range.min, true);
+    ok(groupItem.isStacked(), "groupItem is stacked");
+
+    // one step back to un-stack the groupItem
+    groupItem.setSize(range.max, range.max, true);
+    ok(!groupItem.isStacked(), "groupItem is no longer stacked");
+
+    // check that close buttons are visible
+    let tabItem = groupItem.getChild(0);
+    isnot(tabItem.$close.css("display"), "none", "close button is visible");
+
+    finish();
+  });
+}

browser/base/content/test/tabview/browser_tabview_expander.js

@@ -17,7 +17,7 @@ function onTabViewWindowLoaded(win) {
   // let's create a group small enough to get stacked
   let group = new contentWindow.GroupItem([], {
     immediately: true,
-    bounds: {left: 20, top: 300, width: 300, height: 300}
+    bounds: {left: 20, top: 300, width: 400, height: 400}
   });
 
   let expander = contentWindow.iQ(group.container).find(".stackExpander");