Closed Bug 596256 Opened 14 years ago Closed 14 years ago

DOM for TabView doesn't represent relationship between tab groups and contained tabs

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: whimboo, Unassigned)

References

Details

(Whiteboard: [mozmill])

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

While working on the API for our Mozmill Panorama tests I got some problems yesterday by identifying which tabs belongs to which groups. Right now there is no relationship you can easily get out of the DOM. Tabs are always added as siblings to the groups and not as child nodes. While this is probably fine from a visualization standpoint, the hierarchy is broken. Tabs which are part of a group should really be child nodes of the appropriate group node. Tabs which do not belong to a group should be at the same level as the groups.

As a workaround for now, I will have to use the real TabView object. Getting the list of tabs for a group, all tabs have to be iterated and checked if the parent is the group the test is looking for:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-tabview.js#140

A change like proposed above would also eliminate the needs of a tabInGroupItem class.
Blocks: 596264
Unfortunately, this is a wontfix. Re-parenting DOM elements is a heavy action that breaks event handlers, among other things. We were originally looking at doing an appropriate hierarchy but it just didn't fly in practice.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
I (while I would love to see it work the other way) agree with Aza's wontfix. :/

(In reply to comment #0)
> Getting
> the list of tabs for a group, all tabs have to be iterated and checked if the
> parent is the group the test is looking for:

This shouldn't be necessary.

You should also be able to use the TabItem's iframe's window object's GroupItems object. GroupItems.groupItems is an array of GroupItem objects, each of which have their tab items accessible via getChildren. So if you can easily identify the group item which is the target of your task, you should be able to simply loop over the GroupItems.groupItems to find the appropriate one, then get an array of all the tabs under that group immediately.
> (In reply to comment #0)
> You should also be able to use the TabItem's iframe's window object's
> GroupItems object. GroupItems.groupItems is an array of GroupItem objects, each
> of which have their tab items accessible via getChildren. So if you can easily
> identify the group item which is the target of your task, you should be able to
> simply loop over the GroupItems.groupItems to find the appropriate one, then
> get an array of all the tabs under that group immediately.

Sad to see that decision because it will not allow us to feed the Mozmill tests with the data we really want to test. While this approach is fine, it does *NOT* test the UI we are targeting with Mozmill. That means the UI can be broken while the objects, you are mentioning above, have been created correctly. It's more an unit test then.
I disagree: even if we had gone the other way, just because the dom structure was right wouldn't mean the visual output is right. What you'll want is a function to make sure that a groups tabs are in fact visually inside of the group.
Note Aza, Henrik and I talked on IRC and filed bug 596450 so we can sort of meet halfway.
(In reply to comment #4)
> I disagree: even if we had gone the other way, just because the dom structure
> was right wouldn't mean the visual output is right. What you'll want is a
> function to make sure that a groups tabs are in fact visually inside of the
> group.

Correct. That would be the final goal. But even with those checks we can't make sure everything is displayed correctly. Mozmill isn't a framework we can run reftests with. So yeah, lets find a good set of tests for now which will help you to get a better test coverage. I already have a good set in mind.
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.