DOM attribute for the tab node needed which references the tabgroup

VERIFIED FIXED in Firefox 9

Status

Firefox Graveyard
Panorama
P2
normal
VERIFIED FIXED
7 years ago
a year ago

People

(Reporter: whimboo, Assigned: raymondlee)

Tracking

Trunk
Firefox 9
Dependency tree / graph

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
+++ This bug was initially created as a clone of Bug #596256 +++

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b6pre) Gecko/20100913 Firefox/4.0b6pre

As talked on bug 596256 we need a way to be able to detect in which tabgroup a tab is contained. I talked with Mitcho on IRC and we came up with a solution that we can add a DOM attribute to any of the tabs which references the tabgroup. That would solve all of our problems and we could implement real ui testing with Mozmill.

Thanks Mitcho for that idea!
Note, to be specific, the idea is to add a DOM attribute (data-group attribute?) to all div.tab elements with the group id. Then also have an attribute on div.group elements which encode the group id's.
Priority: -- → P3

Comment 2

7 years ago
Henrik, I'm assigning this to you. Please unassign if you aren't the right person. I'm also moving this to post Firefox 4, because I don't think we will block on it? That said, I'd love to get the fix in.

Feel free to disagree and we'll discuss.
Assignee: nobody → hskupin
Priority: P3 → P2
Target Milestone: --- → Future
(Reporter)

Comment 3

7 years ago
Sadly I don't have time to work on code for TabCandy. I would love to see a fix for Firefox 4, but given other higher priority bugs on your side and our workaround, this is probably a future targeted one. :/

I don't think that this is a too hard to implement patch. So lets put the good first bug entry into the whiteboard.
Assignee: hskupin → nobody
Whiteboard: [good first bug]
(Assignee)

Comment 4

6 years ago
Created attachment 558261 [details] [diff] [review]
v1
Attachment #558261 - Flags: review?(tim.taubert)
(Assignee)

Updated

6 years ago
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Comment on attachment 558261 [details] [diff] [review]
v1

Review of attachment 558261 [details] [diff] [review]:
-----------------------------------------------------------------

Is this still wanted/needed? I'd rather have bug 616576 implemented but that's a lot of work, sadly.

We need to unset/remove the "data-group" attribute once the tab gets removed from a group.
Attachment #558261 - Flags: review?(tim.taubert)
(Assignee)

Comment 6

6 years ago
Created attachment 558365 [details] [diff] [review]
v2

> We need to unset/remove the "data-group" attribute once the tab gets removed
> from a group.

Fixed.
Attachment #558261 - Attachment is obsolete: true
Henrik, would you still benefit from this patch? Do you want us to land it?
(Reporter)

Comment 8

6 years ago
Functional testing wise it would be a great improvement for us. It would make our life way easier to retrieve the list of tabs for a given tabgroup by simply using querySelector with the tabgroup class name.
(In reply to Henrik Skupin (:whimboo) from comment #8)
> Functional testing wise it would be a great improvement for us. It would
> make our life way easier to retrieve the list of tabs for a given tabgroup
> by simply using querySelector with the tabgroup class name.

Ok, cool, this is an easy fix and doesn't cause too much hassle. Let's do it.
(Assignee)

Comment 10

6 years ago
Comment on attachment 558365 [details] [diff] [review]
v2

Any additional things we need to add in this patch?
Attachment #558365 - Flags: review?(tim.taubert)
Comment on attachment 558365 [details] [diff] [review]
v2

Review of attachment 558365 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Attachment #558365 - Flags: review?(tim.taubert) → review+
(Assignee)

Comment 12

6 years ago
Created attachment 558755 [details] [diff] [review]
Patch for checkin

Pushed to try and waiting for the results
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&tree=Try&rev=3947d2a47ffb
Attachment #558365 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/integration/fx-team/rev/43f1efa4b17d
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed-in-fx-team]
Target Milestone: Future → ---
(Assignee)

Updated

6 years ago
Blocks: 596504

Comment 14

6 years ago
https://hg.mozilla.org/mozilla-central/rev/43f1efa4b17d
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][fixed-in-fx-team] → [good first bug]
Target Milestone: --- → Firefox 9
(Reporter)

Updated

6 years ago
Blocks: 686895
(Reporter)

Comment 15

6 years ago
Looks great with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110915 Firefox/9.0a1
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.