Attributes array doesn't populate classes

+++ b/ctools.module
@@ -758,6 +758,12 @@ function ctools_process(&$variables, $hook) {
+    $variables['attributes_array']['class'] = $variables['classes_array'];

Reopening this for a second since I noticed some backwards incompatible changes. This overwrites completely the class key in the attributes_array array. This means that preprocessors that have been using that to pass classes (for whatever reason) are losing all their classes. And (maybe this is just me) but when you hear "attributes" you expect that whatever goes in there will be run through drupal_attributes().

I propose, at least for now, to merge the arrays to make sure classes are not being lost. Attached a patch that does this.

Cheers

This just bit us with some disappearing classes after upgrading to 7.1.10.

I can confirm that the patch in #6 fixes the issue.

Also ran into that issue with classes missing after updating a dev site to ctools 7.1.10
can also confirm #6 fixes the issue

I took a closer look on the Patch in #6 and the code looks good to me

Setting to RTBC and Major since depending on the theme it can break things
also updated the Title to make other people that run into this issue after an ctools update find it more easy.

+1

Debugging this, I've seena good amount of back and forth between classes, attributes and their _array versions.

Omega is also changing a lot here, the core template does print out $classes, it's omega that merges that into attributes again and only prints that. So the bug that we are seeing is maybe only caused by using omega, and @japerry might not have done that?

Patch #6 works for us.

Thank you @Upchuck and @spuky. The recent ctools update would have broken the banner on our site. Patch #6 makes it good again.

I actually just found another regression caused by this that is not fixed with the merge.

alpha_alpha_preprocess_html() removes standard sidebar classes from core but only only removes them from attributes, since its templates only use that.

This merge re-ads those classes, which caused another visual regression for us.

Edit: We are on a very old version of omega. It is possible that this is now different. but I don't think so.. ?

Yeah...I agree that my fix is relatively temporary, however examples like the Omega you bring up Berdir are always gonna be found. I think the problem is that nobody actually has been adding classes in a consistent way due to all the confusion :) For instance I never understood the need for the classes array when there is an attributes array. 100% superfluous in my opinion. But fixing that is a job way bigger than this issue here, nor I think it's actually even worth the time in D7.

What I would do is *try* to determine which approach has the least impact and go with that. The affected people will just have to adjust a bit their preprocessors when upgrading. 2 cents.

Yes, it is a mess.

I'm not saying your patch is not OK (I would have put it back in that case), just wanted to point this out in case others have similar issues. The only way to "fix" this would be the revert the original patch completely and live with whatever bugs existed there.

I updated a number of sites recently, and today I was notified of a "missing classes" issue on a number of the updated sites. I came across this thread by combing through the release notes of all the updated modules (thank God "c"tools is toward the front of the alphabet) after failing to find anything relevant via google search.

The patch from #6 fixed our issue and we are using an Omega based theme. Thanks for the patch!

Same thing here.
Lost all classes in panel regions and of course.. a lot of UI crashed.
#6 serves as an OK workaround, but the whole thing around different locations of those classes requires a redesign at a higher level.

I disagree that it needs a complete redesign, it seems fine that there would just be two options to use in your templates. `classes_array` would populate `class` or `classes`, and `attributes_array` would populate `attributes`.

I've used this with other variables in preprocess and process as well. It seems very illogical to me that they would somehow be combined, and, also, is breaking all of our sites. haha.

The patch works ok, but I think we're going to rollback. I hope there is a revert instead.

+1 for patch #6

ctools screwed up my menu, applying the patch fixed it (together with an obvious cache clear). I'm using Omega 7.x-3.1 as base theme.

I apparently reported a similar bug report, https://www.drupal.org/node/2791857#comment-11576309. I applied each of the patches mentioned in this ticket, instead of mine, but noticed that some classes were still missing. In my opinion, these patches merge the arrays to late in the proces.

Applied patch #6 against ctools 1.10 and all the issues with classes are gone.

Many thanks!

drupal 7.5
omega theme 3.1

Actually found another issue. views-view-unformatted uses classes_array as strings per row id. This breaks that somehow, resulting in a lot of notices for code that relies on it and breaks those classes.

Really wondering what the purpose of the original patch was and what bug it exactly fixes and if it shouldn't just be reverted.

That information above might not be correct, I think something else is causing the notices. Still, I don't think this logic works correctly for that template.

I've just come across this issue after updating ctools to 1.10 on a site with an Omega theme. It worked perfectly before with 1.9

The patch in #6 helps but it can cause other issues.

In my case preprocess_html in theme.inc adds in sidebar info but Omega has moved the content it uses to detect this from ['page']['sidebar_first'] to ['page']['content']['content']['sidebar_first'].

To get around this Omega removes these classes and if you want them then you can add them and others if needed back in using mytheme_preprocess_html

The unpatched ctools 1.10 seems to add the standard classes back in but not the custom ones added by mytheme.

The patched version merges the two class array so you do have both however that means that the core adds the class 'no-sidebars' and my custom theme adds it's own class which could be 'sidebar-second'. These two classes then clash so ideally another solution is needed.

One temporary solution for affected sites using an Omega theme that's adding it's own classes in preprocess_html is to use different classes that don't clash with the core Drupal ones. eg use 'omega-no-sidebars' instead of 'no-sidebars' etc

This will still leave the core Drupal classes in the body tag such as 'no-sidebar' which is messy but they can then just be ignored. This method would work with 1.9 or the patched ctools 1.10 so from a personal point of view I'd either like to see it reverted to 1.9 for this issue or the patch for #6 to be applied.

I imagine this issue will affect a number of other people too.

# 6 Works fine for me. Thanks!

The Omega theme pages was falling apart before this patch was run on Ctools.
- All CSS tags who was relate to Title or Menu-titles-name where gone in the "css-class-tag" for the parent and child menus.

I think this patch makes it urgent for update to 1.11 version of Ctools. I believe that many more is using CSS tag-names who is related to page or menu titles names...

Great thanks, this patch saved ours for me ;)

In my case reverting the original patch was the only way to restore the expected behaviour on a fairly complicated drupal commons install. #6 got partially the way there, but I suspect there were some aspects of the site which relied on the buggy behavior.

I tried patch at #6 but it failed to apply (to ctools-7.x-1.x-dev). I ended up resolving the problem with the patch (#5) on this related/duplicate issue: https://www.drupal.org/node/2791857#comment-11576309

Sorry - I was mistaken: the patch (#5) DOES apply cleanly. I was applying on wrong version of ctools (too many local environments ; )

And this patch DOES fix my problem.

I'm not sure which patch is best or which one may get accepted into the next stable release, but I guess too many solutions is a good problem to have.

Thanks for your work on this.

Confirmed that #26 worked in our installation with an Omega-based theme.

I'm trying to help japerry get ctools-1.11 out the door.

Patch applies cleanly to 7.x-1.x.

Looking at this issue and the comments on it it would seem that some more "+1" on this, especially from omega-based themes, would be good. I can definitely see the scope in this patch for the issues that Berdir mention in #13 and Ollie222 mentions in #24.

In the same function (ctools_process()) is
$classes = drupal_static('ctools_process_classes', array());

which phpcs is complaining should be:
$classes = &drupal_static('ctools_process_classes', array());

.. it appears that $classes is not written in this function, so perhaps this is a benign problem, but it seems to me to be bad practice. A new issue?

I'm using subthemes of Omega 3.1 on several sites, including one that began as a Commerce Kickstart distribution and was subsequently surgically separated from the profile. The patches at #6 and #26 both work for me, however as previously noted the patch at #6 results in a .no-sidebars body class when I do have one or two sidebars in some cases.

Given there are a lot of Omega 3.x subthemes out there, my gut feeling would be that #26 reverting the original commit is probably safer. If the original issue "Attributes array doesn't populate classes" was always true before that commit, then at least nothing is broken that wasn't always that way?

I agree. If many Omega based themes are affected by #6, it's best to revert to how it was before this issue. I think #6 provides a more sane approach overall, but since so many things rely on the broken behaviour it's not worth spending so much time fixing all these things for Drupal 7. Better we focus on D8.

As for $classes = drupal_static('ctools_process_classes', array());, that is definitely incorrect. However, fixing that may introduce the same kind of problem as this issue. We might have functionality relying on it not working, causing a fix to disturb a lot of D7 sites. So perhaps the same reasoning would go for keeping it "broken" :)

I'm on an Omega 3-based theme and #26 seems like the right approach to me. We had to revert the module initially because this was so critical to our operation.

+1 for comment 26

Upgraded a bunch of modules on a site, and the theme explodes..

Applying patch to ctools 1.10 fixed it. Ctools guys please don't break our themes.

Thanks to all those adding their experience.

It seems to me that the best way forward is to apply the reverting patch from #26.

We had to emergency migrate sites to a new multi-site with ctools 1.10. All these sites had Omega as a child theme, and the upgrade threw the classes off (including some that were coming from the context module). I am confirming that patch #26 worked for us!

For those confused about the logic behind $variables['attributes_array'], check function theme() documentation which says:

template_process(&$variables, $hook): Creates an additional set of default variables for all theme hooks with template implementations. The variables created in this function are derived from ones created by template_preprocess(), but potentially altered by the other preprocess functions listed above. For example, any preprocess function can add to or modify the $variables['attributes_array'] variable, and after all of them have finished executing, template_process() flattens it into a $variables['attributes'] string for convenient use by templates.

So I guess the safe way is to add to $variables['attributes_array'], and not modify (i.e. reset) its value.

We use patch 2645612-fix-classes-array-theme-way.patch on production, on a theme derived from Basic and using some THEME_preprocess_HOOK functions in its template.php file. Some of those functions are setting $vars['attributes_array']['class']. Without the patch these customizations were reset by ctools_process().

Applied patch from #36 and it fixes the issue. Thanks.

I had an issue with this, tried applying the patches but non of them seemed to work so i went around and made a solution that so far has been working (still in dev stage).

As i am pretty much new to Drupal i don't know if this solution is up to Drupal standard so someone more experienced could confirm if this helps and/or is a good solution for this problem.

Looks like the array was actually and object and the merge cant be done, so i just typecast it to array.

 // Update the classes within the attributes array to match the classes array
  if (isset($variables['attributes_array']['class'])) {
    $variables['attributes_array']['class'] = array_unique(array_merge($variables['classes_array'], (array) $variables['attributes_array']['class']));
    $variables['attributes'] = $variables['attributes_array'] ? drupal_attributes($variables['attributes_array']) : '';
  }

I'm facing a similar problem and none of the patch fixed it.
After upgrading to 1.11 version, my slider stopped working with the following error messages:

Warning: array_merge(): Argument #2 is not an array in ctools_process() (line 763 of C:\Users\Fabio\Sites\didando-rocks\sites\all\modules\ctools\ctools.module).
Warning: array_unique() expects parameter 1 to be array, null given in ctools_process() (line 763 of C:\Users\Fabio\Sites\didando-rocks\sites\all\modules\ctools\ctools.module).

This is the line 763 of ctools.module:

$variables['attributes_array']['class'] = array_unique(array_merge($variables['classes_array'], $variables['attributes_array']['class']));

I did a dpm($variables) and actually $variables['attributes_array']['class'] is not an array but a string variable.
I tried to change the line 763 with the following one and the error disappeared (but obviously I don't want to hack the ctools.module):

$variables['classes_array'][] = $variables['attributes_array']['class'];

Any suggestion?

Thank you very much,

Fabio

Process question.

Patch #26 was proposed a month ago and confirmed in posts #28, #30, #32, #33 and #35. A simple summary of the proposed action was provided in #34.

Patch #36 was proposed a few days ago and confirmed only in #37. Yet _that_ patch was committed. Why?

+1 for Robert_T in #42, this is bad design and doesn't make sense in theory.

This new issue seems to be related to the patch that got committed (in #38)
2820042

#2820042: Warning: array_merge(): Argument #2 is not an array in ctools_process

Updated revert patch. Happy to work towards a solution that fixes this issue and maintains BC, but again, this works for now.

After upgrading ctools my whole site/theme was utterly broken, since all the classes I was using to target blocks were missing! Patch in #47 un-breaks everything, so marking as RTBC. Thank you @Sam152 for the patch.

I see that issue solved in the latest Ctools release 7.x-1.11
https://www.drupal.org/project/ctools/releases/7.x-1.11
Marking it fixed.

@alesr: Please read the entire thread before closing this issue. It is not closed! The correct patch is awaiting action.

IMO, the wrong patch was committed in 1.11. The correct patch to be committed is patch #47 (== patch #26). See my question about why the committed patch was committed at #42. @jenlampton also confirmed that patch #47 is the correct solution in #48.

I was running into an error as well:
Warning: array_merge(): Argument #2 is not an array in ctools_process (line 763...

I can confirm that patch #47 solved the issue for me. Thank you @Sam152.

Running into this error as well. The errors gone after applying patch #47.

Warning: array_unique() expects parameter 1 to be array, null given in ctools_process() (line 763 of /public_html/sites/all/modules/ctools/ctools.module).

Warning: array_merge(): Argument #2 is not an array in ctools_process() (line 763 of /public_html/sites/all/modules/ctools/ctools.module).

Running into that error is an indication of the misuse of the attributes array. Somewhere probably in your code you are adding a string to the "class" key instead of an array of strings.

My classes also disappeared upon updating from ctools-7.x-1.9 to ctools-7.x-1.10
#6 works all fine for me and my classes array came back after applying the patch - tested thoroughly and not getting any module / theme related errors as per several comments I saw around Omega (which I'm not using) I'd say that #6 too is acceptable on its own and not compromising oncoming array of classes (and at some point joins #47).

Post update to 7.x-1.9 to 7.x-1.10 produce the same error, classes are missing.

As per comment : https://www.drupal.org/node/2645612#comment-11740157. Using latest version 7.x-1.11 resolves issue and missing classes are back with Omega theme. Tried patch #47 with Omega also resolves the issue without giving any error.

I was running version 7.x-1.11 and was getting the error. Patch #47 worked great.

Thanks! :-)

This wasn't added to CTools 1.12, so hopefully 1.13..

Thank you patch #47. You're a rock star! (updated to 1.12 hoping for the fix. Applying the patch did the trick in the interim).

thanks a lot for patch #47.. works like charm with Drupal 7.53 and CTools 7.x-1.12 ...

Why where the changes reverted? What needs to be fixed?

@Liam Morland it was reverted due to comment #15 likely #2645612-15: Attributes array doesn't populate classes