Bug 567029 - "Titles of tab items get clipped off within groups" [f=ian r=dietrich a=blocking]
authorSean Dunn <seanedunn@yahoo.com>
Mon, 27 Sep 2010 01:25:00 -0500
changeset 61332 e8a37c179e5e6c099825a3c48870dd0be34e920e
parent 61331 c83167f1e7da6af53fb39e193ce9d7ba530bffb0
child 61333 89b83442d8c946ddc81af6cd4b4c8803aab7980c
push id18316
push usereakhgari@mozilla.com
push dateWed, 26 Jan 2011 19:02:05 +0000
treeherdermozilla-central@908ae37abec8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdietrich, blocking
bugs567029
milestone2.0b11pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 567029 - "Titles of tab items get clipped off within groups" [f=ian r=dietrich a=blocking]
browser/base/content/tabview/groupitems.js
browser/base/content/tabview/items.js
browser/base/content/tabview/tabitems.js
browser/base/content/tabview/ui.js
browser/base/content/test/tabview/browser_tabview_bug587503.js
browser/base/content/test/tabview/browser_tabview_layout.js
--- a/browser/base/content/tabview/groupitems.js
+++ b/browser/base/content/tabview/groupitems.js
@@ -20,16 +20,17 @@
  *
  * Contributor(s):
  * Ian Gilman <ian@iangilman.com>
  * Aza Raskin <aza@mozilla.com>
  * Michael Yoshitaka Erlewine <mitcho@mitcho.com>
  * Ehsan Akhgari <ehsan@mozilla.com>
  * Raymond Lee <raymond@appcoast.com>
  * Tim Taubert <tim.taubert@gmx.de>
+ * Sean Dunn <seanedunn@yahoo.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -415,27 +416,30 @@ GroupItem.prototype = Utils.extend(new I
   //
   // Parameters:
   //   rect - a <Rect> giving the new bounds
   //   immediately - true if it should not animate; default false
   //   options - an object with additional parameters, see below
   //
   // Possible options:
   //   force - true to always update the DOM even if the bounds haven't changed; default false
-  setBounds: function GroupItem_setBounds(rect, immediately, options) {
-    if (!Utils.isRect(rect)) {
-      Utils.trace('GroupItem.setBounds: rect is not a real rectangle!', rect);
+  setBounds: function GroupItem_setBounds(inRect, immediately, options) {
+    if (!Utils.isRect(inRect)) {
+      Utils.trace('GroupItem.setBounds: rect is not a real rectangle!', inRect);
       return;
     }
 
+    // Validate and conform passed in size
+    let validSize = GroupItems.calcValidSize(
+      new Point(inRect.width, inRect.height));
+    let rect = new Rect(inRect.left, inRect.top, validSize.x, validSize.y);
+
     if (!options)
       options = {};
 
-    GroupItems.enforceMinSize(rect);
-
     var titleHeight = this.$titlebar.height();
 
     // ___ Determine what has changed
     var css = {};
     var titlebarCSS = {};
     var contentCSS = {};
 
     if (rect.left != this.bounds.left || options.force)
@@ -934,16 +938,17 @@ GroupItem.prototype = Utils.extend(new I
           this._activeTab = this._children[0];
         else
           this._activeTab = null;
       }
 
       item.setParent(null);
       item.removeClass("tabInGroupItem");
       item.removeClass("stacked");
+      item.isStacked = false;
       item.removeClass("stack-trayed");
       item.setRotation(0);
 
       item.droppable(true);
       item.removeSubscriber(this, "close");
 
       if (typeof item.setResizable == 'function')
         item.setResizable(true, options.immediately);
@@ -1063,25 +1068,27 @@ GroupItem.prototype = Utils.extend(new I
         .show()
         .css({
           left: parentBB.width/2 - this.$expander.width()/2
         });
   },
 
   // ----------
   // Function: shouldStack
-  // Returns true if the groupItem should stack (instead of grid).
+  // Returns true if the groupItem, given "count", should stack (instead of 
+  // grid).
   shouldStack: function GroupItem_shouldStack(count) {
     if (count <= 1)
       return false;
 
     var bb = this.getContentBounds();
     var options = {
       return: 'widthAndColumns',
-      count: count || this._children.length
+      count: count || this._children.length,
+      hideTitle: false
     };
     let arrObj = Items.arrange(null, bb, options);
  
     let shouldStack = arrObj.childWidth < TabItems.minTabWidth * 1.35;
     this._columns = shouldStack ? null : arrObj.columns;
 
     return shouldStack;
   },
@@ -1155,55 +1162,55 @@ GroupItem.prototype = Utils.extend(new I
     var count = childrenToArrange.length;
     if (!count)
       return;
 
     var zIndex = this.getZ() + count + 1;
     var maxRotation = 35; // degress
     var scale = 0.8;
     var newTabsPad = 10;
-    var w;
-    var h;
     var itemAspect = TabItems.tabHeight / TabItems.tabWidth;
     var bbAspect = bb.height / bb.width;
 
-    // compute h and w. h and w are the dimensions of each of the tabs... in other words, the
-    // height and width of the entire stack, modulo rotation.
+    // compute size of the entire stack, modulo rotation.
+    let size;
     if (bbAspect > itemAspect) { // Tall, thin groupItem
-      w = bb.width * scale;
-      h = w * itemAspect;
-      // let's say one, because, even though there's more space, we're enforcing that with scale.
-    } else { // Short, wide groupItem
-      h = bb.height * scale;
-      w = h * (1 / itemAspect);
-    }
+      size = TabItems.calcValidSize(new Point(bb.width * scale, -1),
+        {hideTitle:true});
+     } else { // Short, wide groupItem
+      size = TabItems.calcValidSize(new Point(-1, bb.height * scale),
+        {hideTitle:true});
+     }
+
 
     // x is the left margin that the stack will have, within the content area (bb)
     // y is the vertical margin
-    var x = (bb.width - w) / 2;
-
-    var y = Math.min(x, (bb.height - h) / 2);
-    var box = new Rect(bb.left + x, bb.top + y, w, h);
+    var x = (bb.width - size.x) / 2;
+    var y = Math.min(size.x, (bb.height - size.y) / 2);
+    var box = new Rect(bb.left + x, bb.top + y, size.x, size.y);
 
     var self = this;
     var children = [];
     childrenToArrange.forEach(function GroupItem__stackArrange_order(child) {
       if (child == self.topChild)
         children.unshift(child);
       else
         children.push(child);
     });
 
     children.forEach(function GroupItem__stackArrange_apply(child, index) {
       if (!child.locked.bounds) {
         child.setZ(zIndex);
         zIndex--;
 
         child.addClass("stacked");
-        child.setBounds(box, !animate);
+        child.isStacked = true;
+        // Force a recalculation of height because we've changed how the title
+        // is shown.
+        child.setBounds(box, !animate, {force:true});
         child.setRotation((UI.rtl ? -1 : 1) * Math.min(index, 5) * 5);
       }
     });
 
     self._isStacked = true;
   },
   
   // ----------
@@ -1233,30 +1240,32 @@ GroupItem.prototype = Utils.extend(new I
       arrangeOptions = Utils.extend({}, options, {z: 99999});
     } else {
       this._isStacked = false;
       arrangeOptions = Utils.extend({}, options, {
         columns: this._columns
       });
 
       childrenToArrange.forEach(function(child) {
-        child.removeClass("stacked")
+        child.removeClass("stacked");
+        child.isStacked = false;
       });
     }
   
     if (!childrenToArrange.length)
       return false;
 
     // Items.arrange will determine where/how the child items should be
     // placed, but will *not* actually move them for us. This is our job.
     let result = Items.arrange(childrenToArrange, box, arrangeOptions);
-    let {dropIndex, rects} = result;
+    let {dropIndex, rects, columns} = result;
     if ("oldDropIndex" in options && options.oldDropIndex === dropIndex)
       return dropIndex;
 
+    this._columns = columns;
     let index = 0;
     let self = this;
     childrenToArrange.forEach(function GroupItem_arrange_children_each(child, i) {
       // If dropIndex spacing is active and this is a child after index,
       // bump it up one so we actually use the correct rect
       // (and skip one for the dropPos)
       if (self._dropSpaceActive && index === dropIndex)
         index++;
@@ -1638,17 +1647,17 @@ GroupItem.prototype = Utils.extend(new I
   // Returns all children.
   getChildren: function GroupItem_getChildren() {
     return this._children;
   }
 });
 
 // ##########
 // Class: GroupItems
-// Singelton for managing all <GroupItem>s.
+// Singleton for managing all <GroupItem>s.
 let GroupItems = {
   groupItems: [],
   nextID: 1,
   _inited: false,
   _activeGroupItem: null,
   _activeOrphanTab: null,
   _cleanupFunctions: [],
   _arrangePaused: false,
@@ -2079,17 +2088,17 @@ let GroupItems = {
     }
 
     // create new group for orphan tab and new tabItem
     let tabItems;
     let newGroupItemBounds;
     // the orphan tab would be the same as tabItem when all tabs are app tabs
     // and a new tab is created.
     if (orphanTabItem && orphanTabItem.tab != tabItem.tab) {
-      newGroupItemBounds = orphanTabItem.getBoundsWithTitle();
+      newGroupItemBounds = orphanTabItem.getBounds();
       tabItems = [orphanTabItem, tabItem];
     } else {
       tabItem.setPosition(60, 60, true);
       newGroupItemBounds = tabItem.getBounds();
       tabItems = [tabItem];
     }
 
     newGroupItemBounds.inset(-40,-40);
@@ -2357,28 +2366,16 @@ let GroupItems = {
       if (groupItem.hidden)
         groupItem.closeHidden();
      });
 
     this._removingHiddenGroups = false;
   },
 
   // ----------
-  // Function: enforceMinSize
-  // Takes a <Rect> and modifies that <Rect> in case it is too small to be
-  // the bounds of a <GroupItem>.
-  //
-  // Parameters:
-  //   bounds - (<Rect>) the target bounds of a <GroupItem>
-  enforceMinSize: function GroupItems_enforceMinSize(bounds) {
-    bounds.width = Math.max(bounds.width, this.minGroupWidth);
-    bounds.height = Math.max(bounds.height, this.minGroupHeight);
-  },
-
-  // ----------
   // Function: getUnclosableGroupItemId
   // If there's only one (non-hidden) group, and there are app tabs present, 
   // returns that group.
   // Return the <GroupItem>'s Id
   getUnclosableGroupItemId: function GroupItems_getUnclosableGroupItemId() {
     let unclosableGroupItemId = null;
 
     if (gBrowser._numPinnedTabs > 0) {
@@ -2405,10 +2402,22 @@ let GroupItems = {
       if (groupItem) {
         groupItem.$closeButton.hide();
       }
     } else {
       this.groupItems.forEach(function(groupItem) {
         groupItem.$closeButton.show();
       });
     }
+  },
+  
+  // ----------
+  // Function: calcValidSize
+  // Basic measure rules. Assures that item is a minimum size.
+  calcValidSize: function GroupItems_calcValidSize(size, options) {
+    Utils.assert(Utils.isPoint(size), 'input is a Point');
+    Utils.assert((size.x>0 || size.y>0) && (size.x!=0 && size.y!=0), 
+      "dimensions are valid:"+size.x+","+size.y);
+    return new Point(
+      Math.max(size.x, GroupItems.minGroupWidth),
+      Math.max(size.y, GroupItems.minGroupHeight));
   }
 };
--- a/browser/base/content/tabview/items.js
+++ b/browser/base/content/tabview/items.js
@@ -17,16 +17,17 @@
  * the Mozilla Foundation.
  * Portions created by the Initial Developer are Copyright (C) 2010
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  * Ian Gilman <ian@iangilman.com>
  * Aza Raskin <aza@mozilla.com>
  * Michael Yoshitaka Erlewine <mitcho@mitcho.com>
+ * Sean Dunn <seanedunn@yahoo.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -40,17 +41,17 @@
 // **********
 // Title: items.js
 
 // ##########
 // Class: Item
 // Superclass for all visible objects (<TabItem>s and <GroupItem>s).
 //
 // If you subclass, in addition to the things Item provides, you need to also provide these methods:
-//   setBounds - function(rect, immediately)
+//   setBounds - function(rect, immediately, options)
 //   setZ - function(value)
 //   close - function()
 //   save - function()
 //
 // Subclasses of Item must also provide the <Subscribable> interface.
 //
 // ... and this property:
 //   defaultSize - a Point
@@ -59,16 +60,17 @@
 // Make sure to call _init() from your subclass's constructor.
 function Item() {
   // Variable: isAnItem
   // Always true for Items
   this.isAnItem = true;
 
   // Variable: bounds
   // The position and size of this Item, represented as a <Rect>.
+  // This should never be modified without using setBounds()
   this.bounds = null;
 
   // Variable: zIndex
   // The z-index for this item.
   this.zIndex = 0;
 
   // Variable: debug
   // When set to true, displays a rectangle on the screen that corresponds with bounds.
@@ -238,17 +240,17 @@ Item.prototype = {
       }
     };
   },
 
   // ----------
   // Function: getBounds
   // Returns a copy of the Item's bounds as a <Rect>.
   getBounds: function Item_getBounds() {
-    Utils.assert(Utils.isRect(this.bounds), 'this.bounds');
+    Utils.assert(Utils.isRect(this.bounds), 'this.bounds should be a rect');
     return new Rect(this.bounds);
   },
 
   // ----------
   // Function: overlapsWithOtherItems
   // Returns true if this Item overlaps with any other Item on the screen.
   overlapsWithOtherItems: function Item_overlapsWithOtherItems() {
     var self = this;
@@ -429,29 +431,32 @@ Item.prototype = {
         if (data.generation == 0)
           return;
 
         var bounds = data.bounds;
         bounds.width -= sizeStep.x;
         bounds.height -= sizeStep.y;
         bounds.left += posStep.x;
         bounds.top += posStep.y;
-        
+
+        let validSize;
         if (item.isAGroupItem) {
-          GroupItems.enforceMinSize(bounds);
+          validSize = GroupItems.calcValidSize(
+            new Point(bounds.width, bounds.height));
+          bounds.width = validSize.x;
+          bounds.height = validSize.y;
         } else {
-          TabItems.enforceMinSize(bounds);
           if (sizeStep.y > sizeStep.x) {
-            var newWidth = bounds.height * (TabItems.tabWidth / TabItems.tabHeight);
-            bounds.left += (bounds.width - newWidth) / 2;
-            bounds.width = newWidth;
+            validSize = TabItems.calcValidSize(new Point(-1, bounds.height));
+            bounds.left += (bounds.width - validSize.x) / 2;
+            bounds.width = validSize.x;
           } else {
-            var newHeight = bounds.width * (TabItems.tabHeight / TabItems.tabWidth);
-            bounds.top += (bounds.height - newHeight) / 2;
-            bounds.height = newHeight;
+            validSize = TabItems.calcValidSize(new Point(bounds.width, -1));
+            bounds.top += (bounds.height - validSize.y) / 2;
+            bounds.height = validSize.y;        
           }
         }
 
         var pusher = data.pusher;
         if (pusher) {
           var newPosStep = new Point(posStep.x + posStep2.x, posStep.y + posStep2.y);
           apply(pusher, newPosStep, posStep2, sizeStep);
         }
@@ -480,17 +485,17 @@ Item.prototype = {
       } else if (bounds.bottom > pageBounds.bottom) { // this may be less of a problem post-601534
         posStep.y = pageBounds.bottom - bounds.bottom;
         sizeStep.y = -posStep.y / data.generation;
         posStep.y += sizeStep.y;
         posStep2.y = sizeStep.y;
       }
 
       if (posStep.x || posStep.y || sizeStep.x || sizeStep.y)
-        apply(item, posStep, posStep2, sizeStep);
+        apply(item, posStep, posStep2, sizeStep);        
     });
 
     // ___ Unsquish
     var pairs = [];
     items.forEach(function Item_pushAway_setupUnsquish(item) {
       var data = item.pushAwayData;
       pairs.push({
         item: item,
@@ -928,68 +933,72 @@ let Items = {
   //   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.
   //
   // Returns:
-  //   By default, an object with two properties: `rects`, the list of <Rect>s,
-  //   and `dropIndex`, the index which a dragged tab should have if dropped
-  //   (null if no `dropPos` was specified);
+  //   By default, an object with three properties: `rects`, the list of <Rect>s,
+  //   `dropIndex`, the index which a dragged tab should have if dropped
+  //   (null if no `dropPos` was specified), and the number of columns (`columns`).
   //   If the `return` option is set to 'widthAndColumns', an object with the
   //   width value of the child items (`childWidth`) and the number of columns
   //   (`columns`) is returned.
   arrange: function Items_arrange(items, bounds, options) {
     if (!options)
       options = {};
     var animate = "animate" in options ? options.animate : true;
     var immediately = !animate;
 
     var rects = [];
 
-    var tabAspect = TabItems.tabHeight / TabItems.tabWidth;
     var count = options.count || (items ? items.length : 0);
     if (options.addTab)
       count++;
     if (!count) {
       let dropIndex = (Utils.isPoint(options.dropPos)) ? 0 : null;
       return {rects: rects, dropIndex: dropIndex};
     }
 
     var columns = options.columns || 1;
     // We'll assume for the time being that all the items have the same styling
     // and that the margin is the same width around.
     var itemMargin = items && items.length ?
                        parseInt(iQ(items[0].container).css('margin-left')) : 0;
     var padding = itemMargin * 2;
-    var yScale = 1.1; // to allow for titles
     var rows;
     var tabWidth;
     var tabHeight;
     var totalHeight;
 
     function figure() {
       rows = Math.ceil(count / columns);
-      tabWidth = (bounds.width - (padding * columns)) / columns;
-      tabHeight = tabWidth * tabAspect;
-      totalHeight = (tabHeight * yScale * rows) + (padding * rows);
+      let validSize = TabItems.calcValidSize(
+        new Point((bounds.width - (padding * columns)) / columns, -1),
+        options);
+      tabWidth = validSize.x;
+      tabHeight = validSize.y;
+
+      totalHeight = (tabHeight * rows) + (padding * rows);    
     }
 
     figure();
 
     while (rows > 1 && totalHeight > bounds.height) {
       columns++;
       figure();
     }
 
     if (rows == 1) {
-      tabWidth = Math.min(tabWidth, (bounds.height - 2 * itemMargin) / tabAspect);
-      tabHeight = tabWidth * tabAspect;
+      let validSize = TabItems.calcValidSize(new Point(tabWidth,
+        bounds.height - 2 * itemMargin), options);
+      tabWidth = validSize.x;
+      tabHeight = validSize.y;
     }
     
     if (options.return == 'widthAndColumns')
       return {childWidth: tabWidth, columns: columns};
 
     let initialOffset = 0;
     if (UI.rtl) {
       initialOffset = bounds.width - tabWidth - padding;
@@ -1015,22 +1024,22 @@ let Items = {
       
       // record the box.
       rects.push(new Rect(box));
 
       box.left += (UI.rtl ? -1 : 1) * (box.width + padding);
       column++;
       if (column == columns) {
         box.left = bounds.left + initialOffset;
-        box.top += (box.height * yScale) + padding;
+        box.top += box.height + padding;
         column = 0;
       }
     }
 
-    return {rects: rects, dropIndex: dropIndex};
+    return {rects: rects, dropIndex: dropIndex, columns: columns};
   },
 
   // ----------
   // Function: unsquish
   // Checks to see which items can now be unsquished.
   //
   // Parameters:
   //   pairs - an array of objects, each with two properties: item and bounds. The bounds are
@@ -1057,18 +1066,22 @@ let Items = {
         return;
 
       var bounds = pair.bounds;
       var newBounds = new Rect(bounds);
 
       var newSize;
       if (Utils.isPoint(item.userSize))
         newSize = new Point(item.userSize);
+      else if (item.isAGroupItem)
+        newSize = GroupItems.calcValidSize(
+          new Point(GroupItems.minGroupWidth, -1));
       else
-        newSize = new Point(TabItems.tabWidth, TabItems.tabHeight);
+        newSize = TabItems.calcValidSize(
+          new Point(TabItems.tabWidth, -1));
 
       if (item.isAGroupItem) {
           newBounds.width = Math.max(newBounds.width, newSize.x);
           newBounds.height = Math.max(newBounds.height, newSize.y);
       } else {
         if (bounds.width < newSize.x) {
           newBounds.width = newSize.x;
           newBounds.height = newSize.y;
--- a/browser/base/content/tabview/tabitems.js
+++ b/browser/base/content/tabview/tabitems.js
@@ -20,16 +20,17 @@
  *
  * Contributor(s):
  * Ian Gilman <ian@iangilman.com>
  * Aza Raskin <aza@mozilla.com>
  * Michael Yoshitaka Erlewine <mitcho@mitcho.com>
  * Ehsan Akhgari <ehsan@mozilla.com>
  * Raymond Lee <raymond@appcoast.com>
  * Tim Taubert <tim.taubert@gmx.de>
+ * Sean Dunn <seanedunn@yahoo.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -84,16 +85,17 @@ function TabItem(tab, options) {
   this.defaultSize = new Point(TabItems.tabWidth, TabItems.tabHeight);
   this.locked = {};
   this.isATabItem = true;
   this._zoomPrep = false;
   this.sizeExtra = new Point();
   this.keepProportional = true;
   this._hasBeenDrawn = false;
   this._reconnected = false;
+  this.isStacked = false;
 
   var self = this;
 
   this.isDragging = false;
 
   this.sizeExtra.x = parseInt($div.css('padding-left'))
       + parseInt($div.css('padding-right'));
 
@@ -128,17 +130,17 @@ function TabItem(tab, options) {
   this.dropOptions.over = function(e) {
     var $target = iQ(this.container);
     this.isDropTarget = true;
 
     $target.removeClass("acceptsDrop");
 
     var phantomMargin = 40;
 
-    var groupItemBounds = this.getBoundsWithTitle();
+    var groupItemBounds = this.getBounds();
     groupItemBounds.inset(-phantomMargin, -phantomMargin);
 
     iQ(".phantom").remove();
     var phantom = iQ("<div>")
       .addClass("groupItem phantom acceptsDrop")
       .css({
         position: "absolute",
         zIndex: -99
@@ -224,16 +226,25 @@ TabItem.prototype = Utils.extend(new Ite
   forceCanvasSize: function TabItem_forceCanvasSize(w, h) {
     this.canvasSizeForced = true;
     this.canvasEl.width = w;
     this.canvasEl.height = h;
     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-this.sizeExtra.x, true); // in [0,1]
+    return TabItems.fontSizeRange.scale(proportion);
+  },
+
+  // ----------
   // Function: unforceCanvasSize
   // Stops holding the thumbnail resolution; allows it to shift to the
   // size of thumbnail on screen. Note that this call does not nest, unlike
   // <TabItems.resumePainting>; if you call forceCanvasSize multiple
   // times, you just need a single unforce to clear them all.
   unforceCanvasSize: function TabItem_unforceCanvasSize() {
     this.canvasSizeForced = false;
   },
@@ -381,56 +392,60 @@ TabItem.prototype = Utils.extend(new Ite
   //
   // Parameters:
   //   rect - a <Rect> giving the new bounds
   //   immediately - true if it should not animate; default false
   //   options - an object with additional parameters, see below
   //
   // Possible options:
   //   force - true to always update the DOM even if the bounds haven't changed; default false
-  setBounds: function TabItem_setBounds(rect, immediately, options) {
-    if (!Utils.isRect(rect)) {
-      Utils.trace('TabItem.setBounds: rect is not a real rectangle!', rect);
+  setBounds: function TabItem_setBounds(inRect, immediately, options) {
+    if (!Utils.isRect(inRect)) {
+      Utils.trace('TabItem.setBounds: rect is not a real rectangle!', inRect);
       return;
     }
 
     if (!options)
       options = {};
 
-    TabItems.enforceMinSize(rect);
+    // force the input size to be valid
+    let validSize = TabItems.calcValidSize(
+      new Point(inRect.width, inRect.height), 
+      {hideTitle: (this.isStacked || options.hideTitle === true)});
+    let rect = new Rect(inRect.left, inRect.top, 
+      validSize.x, validSize.y);
 
     if (this._zoomPrep)
       this.bounds.copy(rect);
     else {
       var $container = iQ(this.container);
       var $title = iQ(this.nameEl);
       var $thumb = iQ(this.thumbEl);
       var $close = iQ(this.closeEl);
       var $fav   = iQ(this.favEl);
       var css = {};
 
-      const fontSizeRange = new Range(8,15);
-
       if (rect.left != this.bounds.left || options.force)
         css.left = rect.left;
 
       if (rect.top != this.bounds.top || options.force)
         css.top = rect.top;
 
       if (rect.width != this.bounds.width || options.force) {
         css.width = rect.width - this.sizeExtra.x;
-        let widthRange = new Range(0,TabItems.tabWidth);
-        let proportion = widthRange.proportion(css.width, true); // in [0,1]
-
-        css.fontSize = fontSizeRange.scale(proportion); // returns a value in the fontSizeRange
+        css.fontSize = this._getFontSizeFromWidth(rect.width);
         css.fontSize += 'px';
       }
 
-      if (rect.height != this.bounds.height || options.force)
-        css.height = rect.height - this.sizeExtra.y;
+      if (rect.height != this.bounds.height || options.force) {
+        if (!this.isStacked)
+          css.height = rect.height - this.sizeExtra.y - TabItems.fontSizeRange.max;
+        else
+          css.height = rect.height - this.sizeExtra.y;
+      }
 
       if (Utils.isEmptyObject(css))
         return;
 
       this.bounds.copy(rect);
 
       // If this is a brand new tab don't animate it in from
       // a random location (i.e., from [0,0]). Instead, just
@@ -443,29 +458,29 @@ TabItem.prototype = Utils.extend(new Ite
           duration: 200,
           easing: "tabviewBounce",
           complete: function() {
             TabItems.resumePainting();
           }
         });
       }
 
-      if (css.fontSize && !this.inStack()) {
-        if (css.fontSize < fontSizeRange.min)
+      if (css.fontSize && !this.isStacked) {
+        if (css.fontSize < TabItems.fontSizeRange.min)
           immediately ? $title.hide() : $title.fadeOut();
         else
           immediately ? $title.show() : $title.fadeIn();
       }
 
       if (css.width) {
         TabItems.update(this.tab);
 
         let widthRange, proportion;
 
-        if (this.inStack()) {
+        if (this.isStacked) {
           if (UI.rtl) {
             $fav.css({top:0, right:0});
           } else {
             $fav.css({top:0, left:0});
           }
           widthRange = new Range(70, 90);
           proportion = widthRange.proportion(css.width); // between 0 and 1
         } else {
@@ -507,35 +522,16 @@ TabItem.prototype = Utils.extend(new Ite
 
     if (!this.parent && this.tab.parentNode != null)
       this.setTrenches(rect);
 
     this.save();
   },
 
   // ----------
-  // Function: getBoundsWithTitle
-  // Returns a <Rect> for the groupItem's bounds, including the title
-  getBoundsWithTitle: function TabItem_getBoundsWithTitle() {
-    var b = this.getBounds();
-    var $title = iQ(this.container).find('.tab-title');
-    var height = b.height;
-    if ( Utils.isNumber($title.height()) )
-      height += $title.height();
-    return new Rect(b.left, b.top, b.width, height);
-  },
-
-  // ----------
-  // Function: inStack
-  // Returns true if this item is in a stacked groupItem.
-  inStack: function TabItem_inStack() {
-    return iQ(this.container).hasClass("stacked");
-  },
-
-  // ----------
   // Function: setZ
   // Sets the z-index for this item.
   setZ: function TabItem_setZ(value) {
     this.zIndex = value;
     iQ(this.container).css({zIndex: value});
   },
 
   // ----------
@@ -673,34 +669,36 @@ TabItem.prototype = Utils.extend(new Ite
   // Function: zoomOut
   // Handles the zoom down animation after returning to TabView.
   // It is expected that this routine will be called from the chrome thread
   //
   // Parameters:
   //   complete - a function to call after the zoom down animation
   zoomOut: function TabItem_zoomOut(complete) {
     var $tab = iQ(this.container);
-
-    var box = this.getBounds();
-    box.width -= this.sizeExtra.x;
-    box.height -= this.sizeExtra.y;
-
     var self = this;
     
     let onZoomDone = function onZoomDone() {
       self.setZoomPrep(false);
 
       GroupItems.setActiveOrphanTab(null);
 
       if (typeof complete == "function")
         complete();
     };
     
     let animateZoom = gPrefBranch.getBoolPref("animate_zoom");
     if (animateZoom) {
+      let box = this.getBounds();
+      box.width -= this.sizeExtra.x;
+      if (!this.isStacked)
+        box.height -= this.sizeExtra.y + TabItems.fontSizeRange.max;
+      else
+        box.height -= this.sizeExtra.y;
+  
       TabItems.pausePainting();
       $tab.animate({
         left: box.left,
         top: box.top,
         width: box.width,
         height: box.height
       }, {
         duration: 300,
@@ -764,32 +762,36 @@ TabItem.prototype = Utils.extend(new Ite
       // frame #3 (the first frame of real animation). Choosing an animation that starts
       // fast is key.
 
       $div
         .addClass('front')
         .css(this.getZoomRect(2));
     } else {
       let box = this.getBounds();
+
       this._zoomPrep = false;
       $div.removeClass('front');
 
       this.setBounds(box, true, {force: true});
     }
   }
 });
 
 // ##########
 // Class: TabItems
 // Singleton for managing <TabItem>s
 let TabItems = {
   minTabWidth: 40,
   tabWidth: 160,
   tabHeight: 120,
+  tabAspect: 0, // set in init
+  invTabAspect: 0, // set in init  
   fontSize: 9,
+  fontSizeRange: new Range(8,15),
   items: [],
   paintingPaused: 0,
   cachedDataCounter: 0,  // total number of cached data being displayed.
   tabsProgressListener: null,
   _tabsWaitingForUpdate: [],
   _heartbeat: null, // see explanation at startHeartbeat() below
   _heartbeatTiming: 100, // milliseconds between _checkHeartbeat() calls
   _lastUpdateTime: Date.now(),
@@ -802,16 +804,18 @@ let TabItems = {
   // ----------
   // Function: init
   // Set up the necessary tracking to maintain the <TabItems>s.
   init: function TabItems_init() {
     Utils.assert(window.AllTabs, "AllTabs must be initialized first");
     let self = this;
     
     this.minTabHeight = this.minTabWidth * this.tabHeight / this.tabWidth;
+    this.tabAspect = this.tabHeight / this.tabWidth;
+    this.invTabAspect = 1 / this.tabAspect;
 
     let $canvas = iQ("<canvas>")
       .attr('moz-opaque', '');
     $canvas.appendTo(iQ("body"));
     $canvas.hide();
     this.tempCanvas = $canvas[0];
     // 150 pixels is an empirical size, below which FF's drawWindow()
     // algorithm breaks down
@@ -1197,27 +1201,68 @@ let TabItems = {
     var sane = true;
     if (!Utils.isRect(data.bounds)) {
       Utils.log('TabItems.storageSanity: bad bounds', data.bounds);
       sane = false;
     }
 
     return sane;
   },
+  
+  // ----------
+  // 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;
+  },
 
   // ----------
-  // Function: enforceMinSize
-  // Takes a <Rect> and modifies that <Rect> in case it is too small to be
-  // the bounds of a <TabItem>.
-  //
-  // Parameters:
-  //   bounds - (<Rect>) the target bounds of a <TabItem>
-  enforceMinSize: function TabItems_enforceMinSize(bounds) {
-    bounds.width = Math.max(bounds.width, this.minTabWidth);
-    bounds.height = Math.max(bounds.height, this.minTabHeight);
+  // 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;
+  },
+  
+  // ----------
+  // 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;
+      }
+    }
+    return retSize;
   }
 };
 
 // ##########
 // Class: TabCanvas
 // Takes care of the actual canvas for the tab thumbnail
 // Does not need to be accessed from outside of tabitems.js
 function TabCanvas(tab, canvas) {
--- a/browser/base/content/tabview/ui.js
+++ b/browser/base/content/tabview/ui.js
@@ -992,17 +992,17 @@ let UI = {
         case KeyEvent.DOM_VK_UP:
           norm = function(a, me){return a.y < me.y}
           break;
       }
 
       if (norm != null) {
         var nextTab = getClosestTabBy(norm);
         if (nextTab) {
-          if (nextTab.inStack() && !nextTab.parent.expanded)
+          if (nextTab.isStacked && !nextTab.parent.expanded)
             nextTab = nextTab.parent.getChild(0);
           self.setActiveTab(nextTab);
         }
         event.stopPropagation();
         event.preventDefault();
       } else if (event.keyCode == KeyEvent.DOM_VK_ESCAPE) {
         let activeGroupItem = GroupItems.getActiveGroupItem();
         if (activeGroupItem && activeGroupItem.expanded)
--- a/browser/base/content/test/tabview/browser_tabview_bug587503.js
+++ b/browser/base/content/test/tabview/browser_tabview_bug587503.js
@@ -51,33 +51,34 @@ function onTabViewWindowLoaded() {
   ok(TabView.isVisible(), "Tab View is visible");
 
   let contentWindow = document.getElementById("tab-view").contentWindow;
   let [originalTab] = gBrowser.visibleTabs;
 
   let currentGroup = contentWindow.GroupItems.getActiveGroupItem();
 
   // Create a group and make it active
-  let box = new contentWindow.Rect(100, 100, 370, 370);
+  let box = new contentWindow.Rect(100, 100, 400, 430);
   let group = new contentWindow.GroupItem([], { bounds: box });
   ok(group.isEmpty(), "This group is empty");
   contentWindow.GroupItems.setActiveGroupItem(group);
   
   // Create a bunch of tabs in the group
   let tabs = [];
   tabs.push(gBrowser.loadOneTab("about:blank#0", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#1", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#2", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#3", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#4", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#5", {inBackground: true}));
   tabs.push(gBrowser.loadOneTab("about:blank#6", {inBackground: true}));
 
   ok(!group.shouldStack(group._children.length), "Group should not stack.");
-  
+  is(group._columns, 3, "There should be three columns.");
+
   // PREPARE FINISH:
   group.addSubscriber(group, "close", function() {
     group.removeSubscriber(group, "close");
 
     ok(group.isEmpty(), "The group is empty again");
 
     contentWindow.GroupItems.setActiveGroupItem(currentGroup);
     isnot(contentWindow.GroupItems.getActiveGroupItem(), null, "There is an active group");
@@ -148,16 +149,19 @@ function onTabViewWindowLoaded() {
           let currentPos = currentTarget.getBounds().center();
           let targetPos = tabs[5]._tabViewTabItem.getBounds().center();
           // contentWindow.Utils.log(targetPos, currentPos);
           vector = new contentWindow.Point(targetPos.x - currentPos.x,
                                                targetPos.y - currentPos.y);
           // Call with time = 4000
           checkDropIndexAndDropSpace(currentTarget, group, vector.x, vector.y, contentWindow,
                                      function(index, dropSpaceActiveValues) {
+
+            is(group._columns, 3, "There should be three columns.");
+
             // Now: 0, 6, 2, 3, 4, 5, 1
             is(index, 4, "Tab 5 is back and again the fifth tab.");
             contentWindow.Utils.log('dropSpaceActiveValues',dropSpaceActiveValues);
             is(dropSpaceActiveValues[0], false, "The group began by not showing a dropSpace");
             is(dropSpaceActiveValues[dropSpaceActiveValues.length - 1], true, "In the end, the group was showing a dropSpace");
             
             // Get rid of the group and its children
             // The group close will trigger a finish().
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabview/browser_tabview_layout.js
@@ -0,0 +1,146 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is tabview layout test.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ * Raymond Lee <raymond@appcoast.com>
+ * Sean Dunn <seanedunn@yahoo.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+function test() {
+  waitForExplicitFinish();
+
+  // verify initial state
+  ok(!TabView.isVisible(), "Tab View starts hidden");
+
+  window.addEventListener("tabviewshown", onTabViewWindowLoaded, false);
+  TabView.toggle();
+}
+
+let originalGroupItem = null;
+let originalTab = null;
+let contentWindow = null;
+
+function onTabViewWindowLoaded() {
+  window.removeEventListener("tabviewshown", onTabViewWindowLoaded, false);
+  ok(TabView.isVisible(), "Tab View is visible");
+
+  contentWindow = document.getElementById("tab-view").contentWindow;
+
+  is(contentWindow.GroupItems.groupItems.length, 1, "There is one group item on startup");
+  originalGroupItem = contentWindow.GroupItems.groupItems[0];
+  is(originalGroupItem.getChildren().length, 1, "There should be one Tab Item in that group.");
+  contentWindow.GroupItems.setActiveGroupItem(originalGroupItem);
+
+  [originalTab] = gBrowser.visibleTabs;
+
+  testEmptyGroupItem(contentWindow);
+}
+
+function testEmptyGroupItem(contentWindow) {
+  let groupItemCount = contentWindow.GroupItems.groupItems.length;
+
+  // Preparation
+  //
+    
+  // create empty group item
+  let emptyGroupItem = createEmptyGroupItem(contentWindow, 253, 335, 100, true);
+  ok(emptyGroupItem.isEmpty(), "This group is empty");
+
+  is(contentWindow.GroupItems.groupItems.length, ++groupItemCount,
+     "The number of groups is increased by 1");
+
+  // add four blank items
+  contentWindow.GroupItems.setActiveGroupItem(emptyGroupItem);
+
+  let numNewTabs = 4;
+  let items = [];
+  for(let t=0; t<numNewTabs; t++) {
+    let newItem = contentWindow.gBrowser.loadOneTab("about:blank")._tabViewTabItem;
+    ok(newItem.container, "Created element "+t+":"+newItem.container);
+    items.push(newItem);
+  }
+
+  // Define main test function
+  //
+
+  let mainTestFunc = function() {
+    for(let j=0; j<numNewTabs; j++) {
+      for(let i=0; i<numNewTabs; i++) {
+        if (j!=i) {
+          // make sure there is no overlap between j's title and i's box.
+          let jitem = items[j];
+          let iitem = items[i];
+          let $jtitle = contentWindow.iQ(jitem.container).find(".tab-title");
+          let jbounds = $jtitle.bounds();
+          let ibounds = contentWindow.iQ(iitem.container).bounds();
+
+          ok(
+            (jbounds.top+jbounds.height < ibounds.top) || 
+            (jbounds.top > ibounds.top + ibounds.height) ||
+            (jbounds.left+jbounds.width < ibounds.left) || 
+            (jbounds.left > ibounds.left + ibounds.width),
+            "Items do not overlap: "
+            +jbounds.left+","+jbounds.top+","+jbounds.width+","+jbounds.height+" ; "
+            +ibounds.left+","+ibounds.top+","+ibounds.width+","+ibounds.height);        
+        }
+      }
+    }
+
+    // Shut down
+    emptyGroupItem.addSubscriber(emptyGroupItem, "close", function() {
+      emptyGroupItem.removeSubscriber(emptyGroupItem, "close");
+  
+      // check the number of groups.
+      is(contentWindow.GroupItems.groupItems.length, --groupItemCount,
+         "The number of groups is decreased by 1");
+
+      let onTabViewHidden = function() {
+        window.removeEventListener("tabviewhidden", onTabViewHidden, false);
+        // assert that we're no longer in tab view
+        ok(!TabView.isVisible(), "Tab View is hidden");
+        finish();
+      };
+      window.addEventListener("tabviewhidden", onTabViewHidden, false);
+  
+      TabView.toggle();
+    });
+  
+    let closeButton = emptyGroupItem.container.getElementsByClassName("close");
+    ok(closeButton[0], "Group close button exists");
+  
+    // click the close button
+    EventUtils.synthesizeMouse(closeButton[0], 1, 1, {}, contentWindow);
+  };
+
+  mainTestFunc();
+}