Compatibilty with Popups 2 [#488746]

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.

Comment	File	Size	Author
#4	488746-vt-js-check-empty-D6.patch	903 bytes	dave reid
	01-check-settings-before-iterating.patch	532 bytes	DeFr

Comments

Comment #1

quicksketch

he/him

commented 16 June 2009 at 02:38

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.

Comment #2

DeFr commented 16 June 2009 at 16:02

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:

Drupal.behaviors.verticalTabs exists, so it's called by attachBehaviors
Vertical Tabs never added any setting for this page, so Drupal.settings.verticalTabs is null

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.