Closed (fixed)
Project:
Panels
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
29 Aug 2012 at 15:42 UTC
Updated:
16 Jun 2014 at 16:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
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 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 commentedThis patched helped me too when i came across this error in the new Drupal Commons
Comment #5
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 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 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 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 commentedAnd another slight change to avoid notices when new scripts are added.
Comment #12
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 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 commentedThe patch in #11 works for me.
Comment #15
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 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 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 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 commented@mrf the patch in #11 already does alter Panels's info file to specify Drupal 7.23+.
Comment #20
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 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 commentedHow do I install this? I'm new
Comment #24
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 commentedPatch in #22 applies, fixes the problem, and looks good on visual review. Marking RTBC.
Comment #26
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