Compatibilty with Popups 2
DeFr - June 11, 2009 - 14:43
| Project: | Vertical Tabs |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Jump to:
Description
Using the latest alpha version of the Popups module, and trying to load something that doesn't have vertical tabs on it from a node edit form will trigger a JavaScript warning when processing the verticalTabsReload behaviors: when popups.js calls attachBehaviors on the content it has retrieved, Drupal.settings.verticalTabs is null, which makes the very first line of jQuery.each fails trying to get null.length.
Attached patch fixes the problem by returning early in verticalTabsReload if Drupal.settings.verticalTabs isn't set.
| Attachment | Size |
|---|---|
| 01-check-settings-before-iterating.patch | 532 bytes |
#1
This patch seems misplaced. Shouldn't Popups make sure that the Drupal.settings.verticalTabs value has been set? It seems that with the proposed solution Vertical Tabs won't work within the popup.
#2
This patch shouldn't change at all how pages with Vertical Tabs within popups react: for them, Drupal.settings.verticalTabs is correctly filled with the fieldsets that should be rendered as vertical tabs in the popups in this case, so we're not hitting the jQuery.each problem. They don't seem to work right though, so that's something I'll have to check too.
The problem this patch attempts to fix though is the following one: if you have a node form that has Vertical Tabs, and a popup linking to something that can't possibly have Vertical Tabs in it, for example a View, then we're in the following situation when attachBehaviors is called on the popup content:
In this case, calling $.each will make the form error out.
I guess an alternative would be to try to make Popups unregister functions from Drupal.behaviors while attaching behaviors on the popup content, but getting the needed one back would imply reinterpreting every JS files really needed by the popup, which doesn't sound all that good.
#3
The patch works for me, solved my problem in #575566: Drupal.behaviors.verticalTabsReload when no tabs present.