Visibility should be determined by taxonomy_defaults settings, not by taxonomy.module

LinL - August 26, 2009 - 09:46
Project:Taxonomy Defaults
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:sleepcamel
Status:active
Description

Hi,

On nodes with hidden, not active vocabularies, I get these warning messages when previewing the node:

* warning: Illegal offset type in isset or empty in ... modules/taxonomy/taxonomy.module on line 1015.
* warning: Illegal offset type in ... modules/taxonomy/taxonomy.module on line 1016.
* warning: Illegal offset type in ... modules/taxonomy/taxonomy.module on line 1019.
* warning: Illegal offset type in ... modules/taxonomy/taxonomy.module on line 592.

The node goes on to save and everything works as expected, so it is not a major problem, just fills up my log file :)

#1

sleepcamel - August 26, 2009 - 11:16

Please clarify what you mean by a hidden vocabulary.

Are you using the Taxonomy Hide module?
Or are you referring to a taxonomy that is not activated for the current node type, but which has taxonomy defaults enabled for it?

Once you've clarified, I'll get this fixed soon.

#2

LinL - August 26, 2009 - 12:00

No, I'm not using Taxonomy Hide (didn't know about that one). I mean a taxonomy that is not activated for the current node type, but which has taxonomy defaults enabled for it.

Thanks for the fast response.

#3

sleepcamel - August 27, 2009 - 02:11
Status:active» postponed (maintainer needs more info)

I'm not able to reproduce this. Could you give detailed steps starting with a new content type and a new vocabulary?

Also, do you have any other taxonomy-related modules turned on?
And, just to confirm, you're using the 6.x-1.x-dev version, not the 6.x-1.0 version, right?

#4

LinL - August 27, 2009 - 07:27
Title:Warning Message in Preview for hidden vocabularies» Warning Message in Preview for not activated vocabularies

I've now set this up on my test system, a fairly minimal install with CCK, Views, Date, Admin menu, no other taxonomy-related modules enabled and no vocabularies set up.

Here's what I did:

1. Set up New Vocabulary "Test" and set Multiple Select and Required on
2. Add 3 terms: Term One, Term Two, Term Three
3. Set up New content Type: "Testwarning" with just Title and body fields
4. Edit taxonomy and add content type "Testwarning"
5. Add new Testwarning node, choose Term One, preview, save, edit, preview - all as expected.
6. Enable Taxonomy defaults (version 6.x-1.x-dev from 2009-Jan-09)
7. Go to Taxonomy, Default terms, tick enabled, choose Term One
8. Edit Taxonomy, turn off Testwarning Content type from content type list
9. Check back in Taxonomy Defaults page and the following message is next to vocabulary:
"The Test vocabulary is not enabled for the Testwarning content type. Default terms will be added to the content without appearing on the add and edit pages."
10. Add new Testwarning node, preview, save all OK and working as expected.
11. Edit node created in 10, preview - this is when warning message appears as above.

#5

sleepcamel - August 27, 2009 - 22:18

Wonderful - I love it when someone actually provides usable repro steps. Thanks!

Now that I've tracked down the cause of this bug, it's confirmed what I've long suspected: that this module's method of handling this scenario (not enabled in Taxonomy settings, enabled in Taxonomy Defaults settings) is very FUD, because it assumes that disabling a vocabulary in the Taxonomy settings only affects visibility. The original maintainer was fortunate that the side effects were very mild, but they could worsen in future versions of Taxonomy. It's also made the UI very confusing, because of the double negative: to enable this very simple feature, you had to disable ("make it not enabled") the vocab in one spot, and then enable it in another ("make it not not enabled").

So here's what I'm thinking for the next version. I'd love some feedback.
1) Require that vocabularies be enabled in the Taxonomy settings in order to set Taxonomy Defaults for that vocabulary & content type.
2) In the Taxonomy Defaults module, instead of the confusing "enabled" checkbox, I'll have a "Lock & Hide" checkbox that causes the same effect in the end: the defaults stick and can't be edited, and aren't visible on forms.

Opinions?

#6

LinL - August 28, 2009 - 10:26

Glad it made sense :)

Yes, the terminology and UI is a bit confusing. I have tried to explain how to use it in the past and struggled a bit! I like the idea of having it all set on the taxonomy defaults page and showing it as enabled in the general taxonomy settings also seems more logical. Not sure about the word "Lock" though. I think of it more as "set automatically without seeing it on node form", but I can't think of a good checkbox type title for that. Maybe see if the usability group has any ideas?

Happy to test new version when it's ready.

#7

sleepcamel - October 18, 2009 - 03:04
Title:Warning Message in Preview for not activated vocabularies» Visibility should be determined by taxonomy_defaults settings, not by taxonomy.module
Assigned to:Anonymous» sleepcamel
Status:postponed (maintainer needs more info)» active

Checking in a fix shortly, in which I've added a new "Visible" option to the taxonomy defaults admin page.

This will present a moderate upgrade challenge, only because it isn't fully automated and instead requires a little user intervention. I didn't want to automate it, because that would mean rather intrusively changing settings that belong to the taxonomy module. Instead of propagating the bad behavior of this module by making big assumptions about how the taxonomy module is being used, I'm restricting the changes to the Taxonomy Defaults module.

What this means for users:

In Taxonomy Defaults 6.x-1.0 and earlier, you could enable taxonomy defaults for a vocabulary and content type even if that vocabulary wasn't enabled for that content type. The resulting behavior was that the vocabulary wouldn't show up in the node/add and node/edit pages, but the default terms would be assigned. The problem with this approach is described in comments above.

Moving forward, Taxonomy Defaults will completely ignore vocabularies that aren't enabled for the current content type, and instead allow you to hide vocabularies by toggling a "Visible" checkbox.

In order to upgrade your "hidden" vocabularies, you'll need to enable the vocabulary/content type relationship on the taxonomy administration pages, and then turn off the associated "Visible" checkbox. The resulting behavior should be the same, but with less likelihood of bugs and conflicts with other modules.

#8

sleepcamel - October 18, 2009 - 03:04
Status:active» fixed

#9

LinL - October 21, 2009 - 12:42
Status:fixed» active

Hi,

I tried to install the latest beta (6.x-1.1-beta2) but got the following errors running update.php:

* Missing argument 2 for variable_get(), called in ... modules/taxonomy_defaults/taxonomy_defaults.install on line 104 and defined in /var/www/drupal6/includes/bootstrap.inc on line 502.

* array_merge() [function.array-merge]: Argument #2 is not an array in /var/www/drupal6/update.php on line 174.

* Invalid argument supplied for foreach() in /var/www/drupal6/update.php on line 338.

Have to run now, but can give more info later.

#10

sleepcamel - October 21, 2009 - 13:12

LinL - I just checked in an untested fix. If you want to pull the latest .install from CVS and confirm, that would be great. Otherwise I'll test it later today and roll another beta

#11

LinL - October 21, 2009 - 17:51

I've followed the handbook page (http://drupal.org/node/321) for getting stuff out of CVS, but am failing dismally and got the version from Nov 2006! As I don't have time to read up all about CVS just now, I'll wait until it appears in available updates and give it a go tomorrow.

#12

LinL - October 22, 2009 - 19:52

Hi sleepcamel,

I've tried the latest dev version (dated Oct 22nd) and I'm still getting errors.

First I tried an update, manually running the 6102 update from update.php. With this I got these errors:

* warning: array_merge() [function.array-merge]: Argument #2 is not an array in /var/www/drupal6/update.php on line 174.
* warning: Invalid argument supplied for foreach() in /var/www/drupal6/update.php on line 338.

The same as before (in #9), but without the first missing argument error.

So I tried uninstalling the module completely and starting again:

Turned it off at admin/build/modules and uninstalled it to delete the tax_def variables.

Checked taxonomy. I have 7 vocabularies that are all used in 2 content types. All active for both content types.

Switched taxonomy defaults back on in admin/build/modules.

Went to admin/content/taxonomy/taxonomy_defaults. I want only one term in one vocab set by default in one content type and I want that hidden. And this same vocab is required for both content types. So, clicked the content type, clicked the hide checkbox and chose my one term from the Default Terms list. Clicked my other content type to make sure nothing was hidden or defaulted there, it wasn't. Hit Save configuration button and got a white screen with just NULL on it.

In the log file at this point are 4 messages:

* Invalid argument supplied for foreach() in ... modules/taxonomy_defaults/taxonomy_defaults.admin.inc on line 92. (3 times)

and

* Cannot modify header information - headers already sent by (output started at ... modules/taxonomy_defaults/taxonomy_defaults.admin.inc:80) in ... includes/common.inc on line 335.

I've checked the variables and they look as if they set OK. There are 14 in all for my 2 content types and 7 vocabs and they are all set like this:

taxdef_nodetype_n = a:0:{}
taxdef_nodetype_n_visible = b:1;

except for the one that I want hidden and defaulted and that is:
a:1:{i:1;s:1:"1";}
b:0;

I tried adding a new node and it seems to work OK.

Hope that all makes sense, let me know if you need any more info. And I like the new interface, it is much more intuitive.

#13

sleepcamel - October 24, 2009 - 20:15

I'm sitting down to work on this now, but just to be sure I'm understanding you:

Everything is OK on a fresh install, correct? It's only the upgrade path that's presenting problems? Edit: Sorry, read that in a hurry. I got it.

Thx

#14

sleepcamel - October 24, 2009 - 23:21

OK, I'm going to break this into four separate problems:
1) The original visibility issue that is this issue. FIXED last weekend.
2) The warning inside taxonomy_defaults.install due to a bad variable_get call. FIXED mid week.
3) The warning(s) in update.php due to a null return value. FIXED an hour ago.

And the one outstanding problem...
4) White screen & log warnings when submitting the admin form.

When you "uninstalled" taxonomy defaults, did you actually run the uninstaller, or just toggle it on and off? I'm trying to figure out whether this is a functional issue or an upgrade issue.

Also, does the problem go away if you purge the settings from the variable table? The "taxdef_nodetype_n = a:0:{}" settings look very suspicious.

Thanks for your patience on this. If you find any new bugs, please file them in separate issues. Combining multiple bugs is confusing for other users.

#15

LinL - October 25, 2009 - 12:42

Hi,

Yes, sorry about that, as I was typing it I thought I should probably be opening separate issues...

To clarify, I went to admin/build/modules/uninstall and ran the uninstall from there.

Will try and test out the latest version later today.

#16

anasynth - November 20, 2009 - 09:22

Hi,

I just wanted to confirm that I'm having the same problem LinL mentioned above:

Went to admin/content/taxonomy/taxonomy_defaults. I want only one term in one vocab set by default in one content type and I want that hidden. And this same vocab is required for both content types. So, clicked the content type, clicked the hide checkbox and chose my one term from the Default Terms list. Clicked my other content type to make sure nothing was hidden or defaulted there, it wasn't. Hit Save configuration button and got a white screen with just NULL on it.

In the log file at this point are 4 messages:

* Invalid argument supplied for foreach() in ... modules/taxonomy_defaults/taxonomy_defaults.admin.inc on line 92. (3 times)

I set things up exactly the same, one hidden term in one vocab set by default in one content type. I clicked save and got the WSOD and also the exact same error message as above pointing me to the same line of code in the same .inc file.

I using the 6.x-1.1-beta2 version. I do have other taxonomy related modules installed but since LinL got the same results on a fresh test site I assume they're unrelated to this problem.

 
 

Drupal is a registered trademark of Dries Buytaert.