Open Bug 921705 Opened 11 years ago Updated 2 years ago

gBrowser.moveTabTo() may lead to inconsistency between selected tabs and their panels

Categories

(Firefox :: Tabbed Browser, defect)

defect

Tracking

()

People

(Reporter: ttaubert, Unassigned)

References

Details

gBrowser.moveTabTo() re-orders tab elements but doesn't touch their panels. Panels are linked to tabs using .linkedPanel property. Now that itself isn't a problem but the xul:tabpanels implementation in tabbox.xml expects the tabs to be in the same order as their panels:

http://hg.mozilla.org/mozilla-central/file/e4cd2242cc7d/toolkit/content/widgets/tabbox.xml#l321

http://hg.mozilla.org/mozilla-central/file/e4cd2242cc7d/toolkit/content/widgets/tabbox.xml#l624

When changing the tabbox.selectedTab property, tabpanels.selectedPanel will be updated as well. The getRelatedElement() methods fetch the linked tab panel by its ID or iterate all panels until they find one with a matching ID.

Now I can't see how we could end up with .linkedPanel being broken but returning the panel with the same index as the tab doesn't seem the right thing to do as it could be the wrong one.

The only way to fix that is by returning null, right? We can't just move panels around as that would destroy the browser bindings. OTOH, moving tabs around is Firefox specific extension to the tabbox, not sure how that would affect other consumers.

(The code has been introduced by bug 552944.)

Dão and Gavin, I hope you're familiar with the code. Any opinion about this?
Flags: needinfo?(gavin.sharp)
Flags: needinfo?(dao)
Sounds like nsIDOMXULRelatedElement only affects accessibility-related functionality, is that right?

I don't know offhand where we would ever hit the !linkedPanelId case. It's probably not possible in Firefox proper, so in theory there may not be a real issue here, but I agree it looks sketchy. If it can never be reached in any case, we could just try removing that code.
Flags: needinfo?(gavin.sharp)
(dashboard cleanup)
Flags: needinfo?(dao)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.