Posted by moritzz on October 26, 2009 at 6:19pm
| Project: | Administration menu |
| Version: | 6.x-1.5 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | PHP 5.3 |
Issue Summary
As noted on #589156: Warnings on PHP 5.3 (and XAMPP) there seams to be an issue when working on PHP 5.3 which triggers the following error:
warning: Parameter 1 to admin_menu_admin_menu() expected to be a reference, value given in includes/module.inc on line 471.
I just tried to analyze the calling function in Drupal 6.14 core and I don't see the need to get the parameter being passed by reference to admin_menu_admin_menu. I just tried to remove the "&" and everything seams to work as expected. (You can reproduce the bug by triggering "Wipe and rebuild" on admin/settings/admin_menu.)
Can you please check and review the attached patch?
Thanks
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| admin_menu-6.x-1.5-admin_menu.inc_.patch | 370 bytes | Ignored: Check issue status. | None | None |
Comments
#1
Hi, came across this while looking into the admin_menu error I was seeing on php 5.3.
Unfortunately, I believe the "&" is needed for PHP 4, and D6 supports PHP 4. Ref a similar discussion regarding the Views module: http://drupal.org/node/452384#comment-2097968
#2
patch works for me (php 5.3)
#3
This variable still needs to be altered by reference. We need to fix the caller and not use module_invoke_all. Please test the following patch.
#4
Yikes. I just spent several hours trying to figure out why the "create content" link was not appearing in the administration menu on fresh installs, and why the Drupal icon menu was not working at all. It looks like this was the culprit. Installed the patch and this did the trick!
#5
Thanks Dave. Works flawlessly.
#6
Please commit this. Thanks. ;-)
#7
Yes, please commit this to all applicable releases.
Thanks
#8
yes, it works on my windows php 5.3 machine, thx
#9
I've tested it on several sites and it works
#10
Tested and confirmed on CentOS 5.3 running PHP 5.3. Thanks!
#11
Thanks for reporting, reviewing, and testing! Committed to 6.x-1.x.
A new development snapshot will be available within the next 12 hours. Note, however, that there most probably won't be another official release for 1.x, but you are safe to use the development snapshot.
#12
Thank you for the patch!
#13
#14
Thanks, Dave, the patch worked for me. Running PHP 5.3.1 =)
#16
Automatically closed -- issue fixed for 2 weeks with no activity.
#17
Thankx it works perfect.
We just need to remove '&' from "admin_menu_admin_menu(&$deleted)" and make it like "admin_menu_admin_menu($deleted)" in "admin_menu.inc" file.
#18
harpreet.sahota, Thanks a lot!!!
#19
usefull！
many modules have same issue。
#20
Thanx so much, @Harpreet
#21
@sun #11: Just curious, why won't there be any more official releases for 1.x?
I understand that the -dev version is stable, but I have clients who prefer to run only official releases. And since shared hosting companies are starting to switch to PHP 5.3 this problem will affect many users of this fantastic module.
#22
It works. Thanks!
#23
@AdrianB @sun I am experiencing the same problems, clients prefer stable versions.
This would be a good candidate for a very small but important update to the stable version?
#24
Hello!
I am a non-coder Drupal beginner.
I've installed locally Drupal on a Windows machine and I'm building a site.
I get all the time the following alert:
Parameter 1 to admin_menu_admin_menu() expected to be a reference, value given en F:\xampp\htdocs\drupal\includes\module.inc on line 483.
I understand that the above mentioned patch offers a solution for a PHP 5.3 flaw. Did I understand well?
I have no idea how to install the patch.
Should I find a tutorial and install the patch or I can live with it (with no consequences for the site I'm building) until a PHP upgrade eventually solve it?
Thanks in advance for your responses.
#25
This patch worked for me on php5.3.2
#26
I'm also curious why this fix won't be ported to an official release. Given that php 5.3 is standard on the newly released ubuntu 10.04 lts, it seems that more and more people will start to come across this, and once hosting platforms upgrade, even more sites will be affected.
#27
@jhedstrom: You go to sites/all/modules/admin_menu (or where you installed it) and invoke this command from a terminal:
wget -O - http://drupal.org/files/issues/615058-adminmenu-php53-D6-1.patch | patch -p0
#28
It's been two months since I asked so I humbly repeat my question:
@sun #11: Just curious, why won't there be any more official releases for 1.x?
#29
Hi, ami7878
Would you please explain how did you patched PHP?
I'm a beginner and I'll appreciate very much your detailed explanation.
Thanks in advance.
#30
--x--
#31
@AdrianB (#28): Your question probably hasn't been answered because 1) it's off-topic (open a new issue) and 2) it was asked in a closed issue (developers don't usually pay attention to closed issues -- open a new issue).
#32
@mrtoner #31: 1) Yes, it was off topic, but it was sun who brought it up in this issue, so I thought it was ok to ask. But I think you're right about 2). And I thought about making this active but I didn't feel that was the right thing to do. I'll think about open a new issue (probably the next time I have to explain to a client why I chosen the dev version instead of the stable one... :)).
#33
FYI: I used this patch to solve the error on my Drupal 6 + PHP 5.3 install, but didn't have the wget command installed. Here's a page that showed clear instructions for doing so:
http://www.bergek.com/2008/03/08/install-wget-on-mac-os-x/
#34
I'd recommend at least applying this patch to a 1.x-dev branch, so that Ubuntu 10.04 users will have something to upgrade too instead of having to find this patch.
#35
Changing status based on #34.
#36
@mcrittenden Was this patch applied to the dev-Release? If not, I don't see the need to close it.
#37
@moritzz: I didn't close it. I changed it to patch to be ported so that the fix could be applied to 6.x-1.x.
#38
@mcrittenden Sorry, I missed the point. No offense and thanks. :)
#39
Patched worked for me - Ubuntu 10.04, D6.16, PHP5.3.
Thanks!
Just curious, was the warning message harmful? I initially thought of it as simply annoying.
#40
@angheloko afaik, it's just a warning, that had no effect one way or other. because of deprecated php behavior.
#41
it's working for me,thanks.
#42
Just to note, the warning can actually break functionality, AFAIK
#43
worked for me
#44
This won't be fixed until the patch is committed to 1.x.
#45
I have made this patch. This is the correct way that keeps the variable reference, so this will work with both php 4 and 5.3 :)
#46
The original patch was committed to 1.x-dev already.
#47
The patch worked for me, be sure to wipe and rebuild the admin_menu afterwards. I'm using PHP Version 5.3.2-1ubuntu4.2.
#48
harpreet.sahota, thank you very much! It's working great now!
#49
Automatically closed -- issue fixed for 2 weeks with no activity.
#50
I have made your suggested changes but still get this error and don't know why? But is it important? The site is functioning on my local computer as I wanted it to also after I needed to download the dev vers. 6.x-2.x-dev from date module.
#51
#52
This hook does not exist in 3.x.
#53
this bug still exists, and i didn't want to open a new ticket :)
warning: Parameter 1 to admin_menu_admin_menu() expected to be a reference, value given in /var/www/drupal_6_dev/includes/module.inc on line 483.
it does not occur consistently. i haven't noticed how to make it occur, but on a new install, it should appear in short order.
#54
ah, just realized the fix is presumably in the dev version
#55
this fix is really old ... PHP 5.3 is common now, so can we please create a new 1.x release?
#56
I've just installed 6.x-3.0-alpha4 and the bug appears to be fixed.
#57
#58
not fixed in 1.5, is it?
#59
It was a bug in 6.x-1.5. And it is fixed in 6.x-1.x-dev.
I've not used 1.x for round about 1.5 years, so I'd have to test it manually before being able to create a new release.
#60
Ok, that indicates that the 3.0-alpha4 release is actually the more stable and supported one (especially for PHP 5.3). Could you make the 3.0-alpha4 release then the recommended one?
#61
You likely want to read the big notice on the project page: http://drupal.org/project/admin_menu
PS: This bug last existed in 6.x-1.5. It does not exist in 6.x-1.x-dev. The proper version is therefore 6.x-1.5.
#62
I understand that 3.x needs help and focus, but I don't understand why the 1.x branch couldn't get another stable release. Sometimes in Drupal it feels like where forever stuck in the -dev version hell with all the negative side effects of that.
I repeat what I said above: I understand that the -dev version is stable, but I have clients who prefer to run only official releases. And since shared hosting companies are starting to switch to PHP 5.3 this problem will affect many users of this fantastic module.
#63
Yep, saw that. I appreciate your hard work but those bugs don't seem to go away anytime soon. The problem is that admin_menu now has 2 unsupported releases, so I suggest that you make the less annoying one (3.x) the recommended one. Especially if you don't want to touch 1.x anymore.
Anyway, I go with 3.0 now and will shut up.
#64
Automatically closed -- issue fixed for 2 weeks with no activity.
#65
Thanks Dave. The patch worked for me. In my case the admin_menu was not showing the drupalicon menu with flush cache, etc. Now it is working.
Kudos to you man....