This is a bit larger one, but highly needed AFAICT. Initially I just wanted to add sensible defaults to category_display, but I soon realized that there's actually no usable code to integrate into: Category/container previews are currently broken in quite horrible way. Not only values are lost from the "Container information" fieldset on previews - the disruption goes so far, that there's even no "Container information" fieldset anymore; the container gets "Category information" instead (not just title - the real thing).

I believe, that this part still waits for a proper 5.x->6.x port, judging from the code. So, my patch does that, addressing all the modules in beta3 package. It's mostly just moving code around between hook_nodeapi(), hook_form_alter(), and form submit handler (plus update to passed variables, and indentations), but there are quite a few of these - that's because 6.x have a bit different workflow on node previews, which is the source of existing problems.

Basically, it boils down to this: hook_nodeapi($op='prepare') fires on new edit page, but doesn't fire on previews anymore. Form submit handler fires on all submits (preview or save or whatever), so this is where half the code needs to go (processing of submitted values between previews, and before save). The other half moves to the top of hook_form_alter (preparation before rendering the form, both new and previewed). The new form submit handler replaces the old data-flattening function (actually, it's only just that function lightly updated, itself). We also got rid of $_POST, which is discouraged in Drupal now - actually, it just naturally went away, without needing any replacement/changes in the new workflow.

There are a few other small fixes, to get the whole thing working. In particular, I had a bit of fun with category_menu checkboxes "Generate menu links for assigned content", where the current implementation was broken - once there was some existing selection, the boxes taken by other containers were not grayed-out anymore (I believe this is the source of yet another confusion I recently saw in queue), and on previews all boxes got checked always (apart from the preview completely not working, so that no-one saw that yet). Also $behavior wasn't stored in the form along with the rest of properties, and wasn't set afresh neither, which is the main reason why container got Category fieldset on preview. Apart from that, I only just cleaned some conditions for consistency (not preparing unnecessary container-specific data on categories, where the corresponding fieldsets are not added later - obvious and simple thing).

As for the category_display defaults - this part is so small, that I included it too (affects more or less the same code). This originated from #409996: Node's asscociated categories don't show up on node and category page and #464678: Category pages not showing nodes, where people had problems with insane defaults for the per-container settings in category_display module. When someone just enables this sub-module on existing site, defaults take over existing containers, as well as [carelessly] created new ones - and these defaults are "everything off", so the site is seemingly broken, previously existing displays no more showing. This is even more of a problem, because the settings don't even show on container node form, being hidden via JavaScript until generated menu items are enabled, so if the user don't dare to experiment with seemingly unrelated settings, he never see the source of the "problem".

So, this patch provides sensible defaults, more or less emulating the behavior we already have without category_display, so that merely enabling the module doesn't mean any unexpected changes to the display. Note that the defaults apply to both new containers, and existing containers without database records (i.e. created before category_display got installed), so the fix shows nicely on existing sites, too. The defaults are: Links on tagged nodes: Yes; Show content listing on category pages: Yes; Show message on empty categories: Yes. All the rest is disabled.

I tested this for quite a while, and I really hope I didn't screw anything. As far as I tested, all basic features and workflows work well (with flawless previews, obviously), and I tested a few less common scenarios too - but honestly, I didn't test everything. I focused on the settings in category/container forms being saved/reloaded/kept through previews correctly - that much I tested, and I believe it's good. Other functionality shouldn't be affected, as I only just moved the code around without actually changing it, and I tested some basic level of that too. Further testing is welcome (I'm going to run this on my test-site as I work on more fixes/adjustments, so I'm likely to see any remaining problems soon, myself).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JirkaRybka’s picture

FileSize
23.83 KB

Rerolled patch, fixing one leftover problem - I didn't catch two instances, where the changed workflow of flattening fieldsets-data changes array keys, so I ran into problems when moving a category to another container, and while editing a node with generated menu links enabled. Now fixed that, too.

Jaza’s picture

Status: Needs review » Fixed

Committed to HEAD. Thanks! See:

#158598: Category port for Drupal 6.x

Status: Fixed » Closed (fixed)

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