From 7079bce4a6c3abfbe555b6763e5b29061fe9d70c Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Thu, 7 Apr 2011 13:00:06 -0700 Subject: [PATCH] Bug 634672 - Minimum tab size is no longer being respected [r=ian] --- browser/base/content/tabview/groupitems.js | 4 +- browser/base/content/tabview/items.js | 1 - browser/base/content/tabview/tabitems.js | 88 ++++++++----------- browser/base/content/test/tabview/Makefile.in | 2 + .../test/tabview/browser_tabview_bug610208.js | 16 ++-- .../test/tabview/browser_tabview_bug624953.js | 28 ++---- .../test/tabview/browser_tabview_bug625666.js | 49 +++++++++++ .../test/tabview/browser_tabview_bug634672.js | 52 +++++++++++ .../test/tabview/browser_tabview_expander.js | 2 +- 9 files changed, 160 insertions(+), 82 deletions(-) create mode 100644 browser/base/content/test/tabview/browser_tabview_bug625666.js create mode 100644 browser/base/content/test/tabview/browser_tabview_bug634672.js diff --git a/browser/base/content/tabview/groupitems.js b/browser/base/content/tabview/groupitems.js index 39b6f6cd592..0b258869338 100644 --- a/browser/base/content/tabview/groupitems.js +++ b/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; diff --git a/browser/base/content/tabview/items.js b/browser/base/content/tabview/items.js index aba1ac15930..05e5e5e7cd9 100644 --- a/browser/base/content/tabview/items.js +++ b/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 which should have a one-tab space left open, used // when a tab is dragged over. diff --git a/browser/base/content/tabview/tabitems.js b/browser/base/content/tabview/tabitems.js index 6f59c2dbb3e..ed53f449444 100644 --- a/browser/base/content/tabview/tabitems.js +++ b/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; } }; diff --git a/browser/base/content/test/tabview/Makefile.in b/browser/base/content/test/tabview/Makefile.in index 033e4650e6d..8731016bc32 100644 --- a/browser/base/content/test/tabview/Makefile.in +++ b/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 \ diff --git a/browser/base/content/test/tabview/browser_tabview_bug610208.js b/browser/base/content/test/tabview/browser_tabview_bug610208.js index 9973b13b39d..b4bb3fe73bb 100644 --- a/browser/base/content/test/tabview/browser_tabview_bug610208.js +++ b/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'); diff --git a/browser/base/content/test/tabview/browser_tabview_bug624953.js b/browser/base/content/test/tabview/browser_tabview_bug624953.js index d552fda4ccf..5a11cfc5a5e 100644 --- a/browser/base/content/test/tabview/browser_tabview_bug624953.js +++ b/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); }); } diff --git a/browser/base/content/test/tabview/browser_tabview_bug625666.js b/browser/base/content/test/tabview/browser_tabview_bug625666.js new file mode 100644 index 00000000000..a1ba7b883d1 --- /dev/null +++ b/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(); + }); +} diff --git a/browser/base/content/test/tabview/browser_tabview_bug634672.js b/browser/base/content/test/tabview/browser_tabview_bug634672.js new file mode 100644 index 00000000000..d79ebd6d5eb --- /dev/null +++ b/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(); + }); +} diff --git a/browser/base/content/test/tabview/browser_tabview_expander.js b/browser/base/content/test/tabview/browser_tabview_expander.js index 99119e6e60f..a47d945e9be 100644 --- a/browser/base/content/test/tabview/browser_tabview_expander.js +++ b/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");