620061 - Restoring more than one column of tabs make the tabs panel size being too small

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Steps to reproduce:
 * Open 5 or 6 tabs
 * Crash fennec (Hit Ctrl + C in the console)
 * relaunch fennec

Actual result:
 * The tabs container size is sized to hold only one tab

Expected result:
 * The tabs container has the right side to hold all the tabs

Comment 1

[Aaron Train [:aaronmt]]

This good for which release?

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/165330d70cc4

This is for the next release, this is a recent regression from adding a super automatic Tab Restore facility when crashing

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

This cause a regression when opening just one tab normally...

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2 + tests

The bug add some tests to ensure we don't regress again

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 499272 [details] [diff] [review]
Patch v0.2 + tests

>diff --git a/chrome/content/tabs.xml b/chrome/content/tabs.xml

>             // XXX we can do better than using a constant here
>             let columnsCount = Math.ceil(this.children.childNodes.length / Math.floor(this.children.getBoundingClientRect().height / (firstBox.height + 4)));
>-            if (this._columnsCount != columnsCount && (firstBox.left != lastBox.left)) {
>-              let width = Math.max(lastBox.right - firstBox.left, firstBox.right - lastBox.left);
>-              width -= this._columnsCount * COLUMN_MARGIN;
>-
>-              this.children.style.width = (width + columnsCount * COLUMN_MARGIN) + "px";
>+            if (this._columnsCount != columnsCount && window.innerWidth != 1) {

Why | window.innerWidth != 1 | ? What is special about "1"?

r+, but I am worried about depending on | window.innerWidth != 1 |

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Add a comment about why we use | window.innerWidth != 1 | too

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/32a162229d63

Comment 8

[Tony Chung [:tchung]]

Need to Verify fix.  Will test with forcing crash later

Comment 9

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Maemo; Linux armv7l; rv:2.0b9pre) Gecko/20101227 Firefox/4.0b9pre Fennec/4.0b4pre ID:20101227013355

Comment 10

[Dão Gottwald [:dao]]

bugspam