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)
Firefox Graveyard
Panorama
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.
Comment 1•14 years ago
|
||
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
Comment 2•14 years ago
|
||
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.
Reporter | ||
Comment 3•14 years ago
|
||
> (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.
Comment 4•14 years ago
|
||
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.
Comment 5•14 years ago
|
||
Note Aza, Henrik and I talked on IRC and filed bug 596450 so we can sort of meet halfway.
Reporter | ||
Comment 6•14 years ago
|
||
(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.
Assignee | ||
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•