596450 - DOM attribute for the tab node needed which references the tabgroup

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

+++ 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!

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

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.

Comment 2

[Aza Raskin [:aza]]

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.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 6

[Raymond Lee [:raymondlee]]

Attachment: v2

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

Fixed.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Henrik, would you still benefit from this patch? Do you want us to land it?

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

(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.

Comment 10

[Raymond Lee [:raymondlee]]

Comment on attachment 558365 [details] [diff] [review]
v2

Any additional things we need to add in this patch?

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 558365 [details] [diff] [review]
v2

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

Looks good!

Comment 12

[Raymond Lee [:raymondlee]]

Attachment: Patch for checkin

Pushed to try and waiting for the results
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&tree=Try&rev=3947d2a47ffb

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/fx-team/rev/43f1efa4b17d

Comment 14

[:Margaret Leibovic]

https://hg.mozilla.org/mozilla-central/rev/43f1efa4b17d

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Looks great with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110915 Firefox/9.0a1