Dependencies were introduced on color module which are not checked and a dependency on a function in the module from the install file is not supported. These errors result in vertical_tabs being impossable to install on a site that has never had it enabled before.

The attached patch moves vertical_tabs_generate_stylesheet() into an include file (attached) which is included from both vertical_tabs.module and vertical_tabs.install before calling vertical_tabs_generate_stylesheet(). It also wraps color.module-dependent code in if (module_exists('color')).

I haven't tested that this actually does the "coloring in", but it at least let me install, and should still do the coloring in on install.

CommentFileSizeAuthor
#17 vertical_tabs_install_load.patch626 bytesquicksketch
#3 355938.patch2.95 KBBevan
#3 generate_stylesheet.inc_.txt1.44 KBBevan
generate_stylesheet.inc_.txt1.44 KBBevan
vertical-tabs-install.patch3.18 KBBevan

Comments

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed

Thanks, I couldn't bring myself to make an include for 20 lines of code, but I committed the hook_install() fix, which is a pretty bad bug. I'll get out a new release shortly.

Bevan’s picture

Bevan commented
Status: Fixed » Postponed (maintainer needs more info)

On install of vertical tabs (with color.module enabled) I get fatal error vertical_tabs_generate_stylesheet() does not exist. vertical_tabs.module has not loaded when hook_intsall() is invoked. This is why I put the function in an include. Alternatively you could load vertical_tabs.module. However this may have undesirable side effects.

Vertical Tabs installs fine if color module is disabled.

Bevan’s picture

Bevan commented
StatusFileSize
new generate_stylesheet.inc_.txt1.44 KB
new 355938.patch2.95 KB

Rerolled the patch. the include is the same.

domesticat’s picture

domesticat commented
Status: Postponed (maintainer needs more info) » Needs review

Patch applies cleanly for me, and allowed me to install vertical tabs on a site that's 1) never had vertical_tabs installed before and 2) had the color module enabled.

Seems clean to me.

matthew petty’s picture

matthew petty commented

Patch in the original post works for me.

355938.patch does not.

Bevan’s picture

Bevan commented

#5, Matthew, What was the error?

radj’s picture

radj commented

Tested the patches and they worked for me.

philsward’s picture

philsward commented

I am guessing the install problem I am having is related to this?

Fatal error: Call to undefined function vertical_tabs_generate_stylesheet() in /home/myaccount/public_html/t/sandbox/sandbox6/sites/default/modules/testing/vertical_tabs/vertical_tabs.install on line 41

Drupal: 6.9
Color: Installed
Theme: Garland
Vertical Tabs: 6.x-1.x-dev-[2009-01-28]

Dunno if the patch was released in the 2009-01-28 version or not...

Update:
Changing to bluemarine, installing vertical tabs then switching back to garland let me install without a problem...

Bevan’s picture

Bevan commented

@ philsward #8. Yes, this patch fixes that bug

ChrisBryant’s picture

ChrisBryant commented
Status: Needs review » Reviewed & tested by the community

I had the same error on install when using the current version of Vertical Tabs. Patch in #3 worked nicely for me.

whatdoesitwant’s picture

whatdoesitwant commented

Patching is to difficult for me.

blakehall’s picture

blakehall commented

Patch in #3 works here too. Thanks!

rsvelko’s picture

rsvelko commented

Guys - lets fix this simple issue!

I've tested the current dev and beta1 versions:
1. dev installs and enables when color is disabled
2. beta1 does not enable/install no matter if color en/disabled

(the problem comes from a function used in the .install but defined in the .module file which is not yet loaded on install time. ..)

the diff betwen dev and beta1 show that only the "if (module_exists("color"))" is new in the install - which causes the difference between 1 and 2 above...

So people who have reviewed the above patches - which is the best to commit?
And maintainers - commit please :)

rsvelko’s picture

rsvelko commented

for a fast-hack to solve this problem - just comment out the 11th line in the install - you''ll lose the interaction with color module...

ChrisBryant’s picture

ChrisBryant commented
Priority: Normal » Critical

Updating to critical since this prevents the module from being installed for a lot of people.

rsvelko’s picture

rsvelko commented
Title: Can't install » Can't install - color module dependancy and an undefined func

title refreshment

quicksketch’s picture

quicksketch
he/him
commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new vertical_tabs_install_load.patch626 bytes

I've committed the attached patch to fix this problem. Sorry about the serious delay folks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.