Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When running PHP 5.4, I am seeing some errors related to how array_diff_assoc() is handling arrays with nested arrays inside of them. The error I am getting is this: Notice: Array to string conversion in panels_cache_object->cache() (line 182 of modules/panels/includes/plugins.inc).
This seems to be related to the problem referenced in #1525176: Using array_diff_assoc() for multilevel arrays in DrupalDefaultEntityController->cacheGet()
Comments
Comment #1
bgm CreditAttribution: bgm commentedRelated Views issue: #1511396: php 5.4 and "Notice: Array to string conversion"
Comment #2
pfrenssenRelated issue to provide a version of array_diff_assoc() with support for nested arrays in core: #1850798: Add a recursive version of array_diff_assoc().
Comment #3
Volx CreditAttribution: Volx commentedHere is a patch using the same approach I used for the views patch using array_diff_key instead of array_diff_assoc (first patch in https://drupal.org/node/1511396#comment-7553219). But the comment holds here too, I don't know if that is the intended behavior, but it is the same (or pretty close) to the current behavior due to the behavior of array_diff_assoc.
Comment #4
Colin @ PCMarket CreditAttribution: Colin @ PCMarket commentedThis patched helped me too when i came across this error in the new Drupal Commons
Comment #5
das-peter CreditAttribution: das-peter commented@Volx I doubt that this approach works. When I debugged it that weren't associative arrays, thus so a key comparison doesn't seem to be sufficient.
array_diff_assoc()
is about comparing the contents and also checking the index.I'd say for the next panels release we just depend on drupal 7.23 which provides
drupal_array_diff_assoc_recursive()
.However, an extended solution would be to provide a replacement function based on #1850798-68: Add a recursive version of array_diff_assoc() in case panels runs with Drupal <7.23 - check second patch.
Comment #6
pfrenssenI definitely prefer the first patch. Core releases are more frequent than Panels releases so providing this fallback function for all eternity seems like overkill.
Comment #7
neclimdulIf you're running 5.4 you'll need the latest versions of core anyways so it doesn't really make sense to provide a backport.
Comment #8
das-peter CreditAttribution: das-peter commentedSo the dependency is preferred - I'm fine with that :)
While testing with testing our installation after the patch I've found errors like:
Notice: Undefined index: data in panels_cache_object->restore() (line 213 of /var/www/drupal/modules/panels/includes/plugins.inc).
I think this could be a side effect of the patch since the diff seems to work differently (correct?) now.
I've added a small change to ensure that js is recognized whether it is declared implicitly as key or explicitly as "data" key in the js array.
^^ Would be good to know if someone else is experiences this issue!
Comment #9
das-peter CreditAttribution: das-peter commentedHmm, I think this change introduced more trouble than expected.
Because
drupal_array_diff_assoc_recursive()
returns only the portions of the array that differ, which can be very granular, while we need to ensure to store the whole portion.My current solution is to iterate over the diff and use
drupal_array_merge_deep()
to merge the diff into the start js and store it that way in the js cache.Comment #10
das-peter CreditAttribution: das-peter commentedAnd here we go again. Because I've unified the storage of the js cache for js-code and js-settings the same has to be done for restoring it. It's as simple as use the 'data' key in the array.
If someone wants an interdiff let me know ;)
Comment #11
das-peter CreditAttribution: das-peter commentedAnd another slight change to avoid notices when new scripts are added.
Comment #12
markbannister CreditAttribution: markbannister commented11 worked for me. For some reason the patch would not apply using the patch command a and I had to apply it manually.
Comment #13
das-peter CreditAttribution: das-peter commentedI've just tested applying the patch against the latest panels 7.x-3.x and I didn't experience any issues. Please let me know if someone else has trouble applying the patch.
Comment #14
pjcdawkins CreditAttribution: pjcdawkins commentedThe patch in #11 works for me.
Comment #15
ptmkenny CreditAttribution: ptmkenny commentedI applied the patch in #11.
In response to #8:
After applying the patch, I got several errors, such as:
I tried to clear the caches with Drush (cc all) and that failed:
However, I then logged in to the site and flushed all caches using the admin menu module. Now I don't get the errors anymore and everything appears to be working fine. So, somehow the patch makes it hard to clear the cache with Drush (immediately after applying the patch, I can revert to the dev version with git and then clear the cache with Drush, but if I re-apply the patch, I cannot). That said, the patch does apply and gives the expected result after the cache is cleared.
Comment #16
pjcdawkins CreditAttribution: pjcdawkins commentedIt works for 4 people.
I suggest perhaps it's problematic to depend on Drupal 7.23+ but I don't see any better solution.
Comment #17
mrf CreditAttribution: mrf commentedFWIW I noticed the same thing as #12 when applying patch. Doing a patch -p1 worked for me second time through.
Patch should be updated to include the 7.23 as the minimum Drupal version in Panel's info file, that's the only way to guarantee people upgrade Drupal first. Failing to do so and this patch causes fatal errors on any panels page.
Comment #18
el1_1el CreditAttribution: el1_1el commentedThe extended patch in #5 seems to work just fine in core v 7.20, in case anyone needs a quick fix
Comment #19
pjcdawkins CreditAttribution: pjcdawkins commented@mrf the patch in #11 already does alter Panels's info file to specify Drupal 7.23+.
Comment #20
richsky CreditAttribution: richsky commented#11 = big white screen for me, Fatal error: Call to undefined function drupal_array_diff_assoc_recursive() were is this function supposed to be found?
Oups, still with 7.22 in prod. Nevermind.
Comment #21
lotyrin CreditAttribution: lotyrin commentedWhy not do as Views has done in #1511396:
Comment #22
jhedstromPatch takes into account views fix mentioned above, removing the need for upping the core version in the .info file.
Comment #23
isloera CreditAttribution: isloera commentedHow do I install this? I'm new
Comment #24
das-peter CreditAttribution: das-peter commented@isloera Take a look here: https://drupal.org/patch/apply
@jhedstrom Thanks for the update! I'd like to set this to RTBC but unfortunately it still contains a lot of code that I wrote. So I'm not independent enough to do so ;)
Comment #25
IshaDakota CreditAttribution: IshaDakota commentedPatch in #22 applies, fixes the problem, and looks good on visual review. Marking RTBC.
Comment #26
Exploratus CreditAttribution: Exploratus commented#22 Worked for me as well. Thanks! Commit?
Comment #27
japerryTested, looks good and fixed:
http://drupalcode.org/project/panels.git/commit/baed7ee