Event Cart: Fix PHP 7.2 fatal error (pass by ref)

[mlutfy]

Overview

To reproduce:

• Use PHP 7.2
• Enable the Event Cart
• Create an Event, add it to the cart, then head to the cart checkout (/civicrm/event/cart_checkout).

Result: blank page / fatal error.

Technical Details

PHP Fatal error:  Only variables can be passed by reference
in [...] CRM/Event/Cart/Form/Cart.php on line 155

I'm not a fan of this function, and this seems like an incorrect usage, but I couldn't figure out how to trigger the error (for some reason, the PHP fatals, even if the code does not run on that line).

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@mlutfy I think the better fix would be not to call that function & just call

CRM_Core_Session::singleton()->setStatus();

We seem to be calling a complex function to no benefit here

[colemanw]

Yes to Eileen's suggestion, and another piece of cleanup we could do is to remove the & in displaySessionError(&$error and getMessages(&$error as it serves no purpose. Just another holdover from php4.

[mlutfy]

Fine by me. I was just hesitant since I don't know how to trigger/test the error.

I'd rather avoid venturing into further changes with displaySessionError, since it seems rather peculiar and hard to test. I can only say that there is now one less place in the code calling it, so that we can one day deprecate it :)

[eileenmcnaughton]

@mlutfy I gave it merge-on-pass but I think the last var should actually be 'error' - just from the fn signature.

   * @param string $type
   *   The type of this message (printed as a css class). Possible options:
   *     - 'alert' (default)
   *     - 'info'
   *     - 'success'
   *     - 'error' (this message type by default will remain on the screen
   *               until the user dismisses it)
   *     - 'no-popup' (will display in the document like old-school)

[mlutfy]

oops, indeed, thanks.

I had randomly copy-pasted, and found a few errors (grep -r setStatus . | grep fail)

./CRM/Mailing/Form/Optout.php:      CRM_Core_Session::setStatus($statusMsg, '', 'fail');
./CRM/Mailing/Form/Unsubscribe.php:      CRM_Core_Session::setStatus($statusMsg, '', 'fail');
./CRM/Mailing/Form/Unsubscribe.php:      CRM_Core_Session::setStatus($statusMsg, '', 'fail');

Fixed in #13943

[eileenmcnaughton]

unrelated fail