Stacks do not show last-visited tab in the front for children after number 6

VERIFIED FIXED

Status

defect
P3
normal
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: bugzilla.mozilla.org, Assigned: ttaubert)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre

Assume that tabs are labeled 1 ... N and 1 ... 6 are shown.
If I open 5 and go back to panorama it does show 5 in front.
If i open 10 it shows 1 in front.

from irc:
<iangilman> our "only show top 6" patch evidently didn't respect .topChild

Reproducible: Always
Reporter

Updated

8 years ago
Version: unspecified → Trunk

Comment 1

8 years ago
Works for me on: 
Mozilla/5.0 (Windows NT 5.1; rv:2.0b12pre) Gecko/20110213 Firefox/4.0b12pre
Reporter

Comment 2

8 years ago
Tested again with Gecko/20110215 Firefox/4.0b12pre and the bug still persists.

Just to reiterate. You have to open the Nth tab (where N > 6) in a stacked group, then go to panorama.
Blocks: 627096
Priority: -- → P3
Summary: Stacks do not show last-visited tab in the front → Stacks do not show last-visited tab in the front for children after number 6
Assignee: nobody → tim.taubert
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Posted patch patch v1 (obsolete) — Splinter Review
Attachment #512778 - Flags: review?(ian)
Comment on attachment 512778 [details] [diff] [review]
patch v1

>+    testTopOfStack(groupItem.getChild(1), function () {
>+      testTopOfStack(groupItem.getChild(6), function () {
>+        closeGroupItem(groupItem, function () {
>+          hideTabView(finish);

That should be finishTest in that last line, yes?

> function createGroupItemWithTabs(win, width, height, padding, urls, animate) {
>   let contentWindow = win.TabView.getContentWindow();
>   let groupItemCount = contentWindow.GroupItems.groupItems.length;
>   // create empty group item
>-  let groupItem = createEmptyGroupItem(contentWindow, width, height, padding, animate);
>+  let groupItem = createEmptyGroupItem(contentWindow, width, height, padding, !animate);

Now that you mention it, why does createEmptyGroupItem have a different API than createGroupItemWithTabs? Seems like it's animate for a lot of those routines, and then it's noAnimation for createEmptyGroupItem. Seems like that's what should be fixed.

Otherwise looks lovely.
Attachment #512778 - Flags: review?(ian) → review-
Posted patch patch v2 (obsolete) — Splinter Review
(In reply to comment #4)
> That should be finishTest in that last line, yes?

Oops :)

> Now that you mention it, why does createEmptyGroupItem have a different API
> than createGroupItemWithTabs? Seems like it's animate for a lot of those
> routines, and then it's noAnimation for createEmptyGroupItem. Seems like that's
> what should be fixed.

Fixed.
Attachment #512778 - Attachment is obsolete: true
Attachment #512960 - Flags: review?(ian)
Comment on attachment 512960 [details] [diff] [review]
patch v2

Looks great!
Attachment #512960 - Flags: review?(ian) → review+
Comment on attachment 512960 [details] [diff] [review]
patch v2

a=beltzner
Attachment #512960 - Flags: approval2.0? → approval2.0+
Attachment #512960 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/a63e2fb78899
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Comment 11

8 years ago
Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110220 Firefox/4.0b12pre

Verified issue and it's no longer present.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.