Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.2] Reintroduce workaround for arguments passed by reference #38000

Merged
merged 3 commits into from
Jun 9, 2022

Conversation

joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This PR revert the change introduced in PR #37178 because it causes some warnings when use HTMLHelper to call a helper method which need parameters passed by reference.

Testing Instructions

  1. Use 4.2 nightly build
  2. Open administrator/templates/atum/index.php , add two lines of code below somewhere in that file:
$children = [];
$list      = HTMLHelper::_('menu.treerecurse', 0, '', [], $children, 9999, 0, 0);
  1. Access to administrator area of the site

Actual result BEFORE applying this Pull Request

Warnings message is displayed line below:

Warning
Parameter 4 to Joomla\CMS\HTML\Helpers\Menu::treerecurse() expected to be a reference, value given in
D:\www\joomla42\libraries\src\HTML\HTMLHelper.php
on line
289

Expected result AFTER applying this Pull Request

No warnings anymore.

@simbus82
Copy link
Contributor

simbus82 commented Jun 7, 2022

Joomla 4 doesn't support php 5 and pass by reference is deprecated since PHP 5. Why re-add some obsolete code in J4.2?

@joomdonation
Copy link
Contributor Author

I'm unsure if the comment PHP 5.3 workaround here is correct. The fact is that some of our helper methods like https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/HTML/Helpers/Menu.php#L422 has parameter passed by reference. And without reverting the PR, calling these methods will cause warnings as I mentioned in PR description

You can try to test the code yourself to see the issue.

@richard67
Copy link
Member

I'm unsure if the comment PHP 5.3 workaround here is correct.

@joomdonation Maybe change that comment to something like pass by reference workaround?

@joomdonation
Copy link
Contributor Author

@richard67 I updated the comment. Hope it is more clear/accurate now.

@laoneo
Copy link
Member

laoneo commented Jun 7, 2022

Then I would adjust the title as it is not a revert anymore.

@toivo
Copy link
Contributor

toivo commented Jun 9, 2022

I have tested this item ✅ successfully on 286ba9b

Tested successfully in Joomla 4.2.0-beta1-dev of 9 June in Wampserver 3.2.9 using PHP 8.0.15


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38000.

@HLeithner HLeithner changed the title [4.2] Revert #37178 [4.2] Reintroduce workaround for arguments passed by reference Jun 9, 2022
@HLeithner HLeithner merged commit f8b855e into joomla:4.2-dev Jun 9, 2022
@HLeithner
Copy link
Member

Thanks

@joomdonation joomdonation deleted the revert_37178 branch June 9, 2022 05:49
@Quy Quy added this to the Joomla 4.2.0 milestone Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants