avoid fatal error in PHP 8 in BAO/Navigation.php

[sebalis]

See https://lab.civicrm.org/dev/core/-/issues/3968 for details.

Overview

Trying to upgrade a test instance of Wordpress+CiviCRM to PHP 8 I found that this line can throw a fatal error. Not sure whether this is the best way to fix it, that’s for core developers to decide.

Before

The CiviCRM menu does not show after an upgrade to PHP 8, due to this line throwing a fatal error “Unsupported operand types: string - string”

After

The menu is back :-)

[civicrm-builder]

Can one of the admins verify this patch?

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

@civicrm-builder add to whitelist

[eileenmcnaughton]

We should target the 5.56 rc branch I think as this appears to be a regression

[sebalis]

Hey @eileenmcnaughton, I don’t know if this is a request for me to change my pull request. I can certainly do that if it helps. As an aside, I would not be sure if my definition of regression includes code that was created 4.5 years ago (and never changed since) breaking in a new language version.

[colemanw]

I agree that technically it's not a regression in our codebase. However, the user will experience it as a regression (it works, they upgrade, it stops working), and it's such a safe & simple fix that we're better off fixing it in the RC. We'll get fewer complaints that way :)
@sebalis you could either change the base of this PR to 5.56 and rebase, or you could open a new PR by editing the file on the 5.56 branch: https://github.com/civicrm/civicrm-core/blob/5.56/CRM/Core/BAO/Navigation.php

[sebalis]

Happy to play around a bit – let’s see how I can mess this up ;-)

[sebalis]

I didn’t find a guide that seemed appropriate for this situation and ended up cherrypicking that one commit onto a newly created patch-1 branch (created at the current head of 5.56), then force-pushing that to patch-1 on my fork. But the web view here still says my request is based on master. Now watiing for the test build to finish to see if it changes then. I refer back to my previous comment …

[colemanw]

@sebalis that worked. The last step was just to edit the PR in GitHub and change the base, which I've done.

[sebalis]

Thanks for helping out. I tried to find rhat setting in the Github web interface, but didn’t succeed.

[colemanw]

It's hidden behind the Edit button at the top right of the screen.

[sebalis]

Of course it is – I did try that button but only noticed that it allowed me to edit the title, not change the base branch :-) :-(

[eileenmcnaughton]

Thanks for this @sebalis - the last step is for you to create a second PR to add yourself to contributors.yml

https://github.com/civicrm/civicrm-core/blob/master/contributor-key.yml

[sebalis]

Thanks @eileenmcnaughton :-) See #24914