Switching among groups always selects the first tab of group

VERIFIED FIXED

Status

defect
P3
normal
VERIFIED FIXED
9 years ago
3 years ago

People

(Reporter: Mardak, Assigned: raymondlee)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [on-panorama-central])

Attachments

(1 attachment, 3 obsolete attachments)

Presing ctrl-` won't go back to the previously selected tab of that group. And if you happen to have an app tab, switching to another group and going to the "previous tab" to the left will hit the bug where selecting the app tab switches to the group.
app tabs problem is bug 578553. let's keep this one solely on restoring group state.
Depends on: 588217
this doesn't actually depend on the session restore support. just need to keep flag somewhere saying what the last-active-tab for each group is, and make it active when the group is opened.
No longer depends on: 588217
Posted patch v1 (obsolete) — Splinter Review
this works in the current session, but doesn't persist the data across restarts yet.
Assignee: nobody → dietrich
Attachment #470836 - Flags: feedback?(ian)
Is the active tab an attribute that only makes sense in the context of Panorama or should it be on xul tabs? I suppose it probably won't be useful for css to match [hidden]:not([active]) tabs differently as they're.. hidden ;)
I don't think it's going to be useful on xul tabs...
Duplicate of this bug: 587602
Posted patch v1 (obsolete) — Splinter Review
oops, forgot to qrefresh.
Attachment #470836 - Attachment is obsolete: true
Attachment #470860 - Flags: feedback?(ian)
Attachment #470836 - Flags: feedback?(ian)
Attachment #470836 - Flags: feedback?(ian)
weird. the feedback flag looked set on the main bug page, but wasn't in the attachment page.
Comment on attachment 470860 [details] [diff] [review]
v1

 looks good to me. I don't know that it needs to save anyway.
Attachment #470860 - Flags: feedback?(ian) → feedback+
Comment on attachment 470860 [details] [diff] [review]
v1

Ok, thanks. Asking for review then, so can push it directly m-c (and t-c if you want).

Will file a separate bug for persisting position across sessions.
Attachment #470860 - Flags: review?(ian)
Comment on attachment 470860 [details] [diff] [review]
v1

canceling request, patch needs test.
Attachment #470860 - Flags: review?(ian)
Priority: -- → P3
Blocks: 597043
Assignee: dietrich → nobody
Going to write a test for this patch.
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Posted patch v1 (obsolete) — Splinter Review
With test
Attachment #470860 - Attachment is obsolete: true
Attachment #477428 - Flags: feedback?(ian)
(In reply to comment #14)
> Created attachment 477428 [details] [diff] [review]
> v1
> 
> With test

Passed try.
Comment on attachment 477428 [details] [diff] [review]
v1

+  is(gBrowser.selectedTab, groupItemOne.getChild(1).tab,
+    "The currently selected tab should be the first tab in the groupItemTwo");

The string should read "second tab" rather than "first tab".

F+ with that fixed. Assigning review to Dolske (Dietrich wrote the original patch).
Attachment #477428 - Flags: review?(dolske)
Attachment #477428 - Flags: feedback?(ian)
Attachment #477428 - Flags: feedback+
Posted patch v1Splinter Review
Fixed the comment suggested by Ian

f+=ian
Attachment #477428 - Attachment is obsolete: true
Attachment #477941 - Flags: review?(dietrich)
Attachment #477428 - Flags: review?(dolske)
Attachment #477941 - Flags: review?(dietrich) → review?(dolske)
Attachment #477941 - Flags: review?(dolske) → review+
Attachment #477941 - Flags: approval2.0?
Attachment #477941 - Flags: approval2.0? → approval2.0+
Attachment #470836 - Flags: feedback?(ian) → feedback+
Landed on mozilla-central: 

http://hg.mozilla.org/mozilla-central/rev/151d90b45e46
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
verified with recent nightly build of minefield
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.