(dev/joomla#10) Remove Joomla-specific error display and use native CiviCRM

[andrewpthompson]

Overview

See https://lab.civicrm.org/dev/joomla/issues/10
CiviCRM fatal errors & unhandled exceptions are rendered very poorly on Joomla due to the inability of Joomla's exception handling to handle HTML in error messages. CiviCRM uses HTML in the error messages, plus inline script and CSS from the template and this is all displayed on the Joomla error page.

Because of these Joomla constraints (we can only use plain text and not even new line characters) it is better to remove the Joomla-specific way of rendering errors and display natively in CiviCRM as for Drupal.

Before

Fatal errors & unhandled exceptions are very dificult to read on Joomla:

After

Fatal errors & unhandled exceptions are rendered natively by CiviCRM:

Technical Details

This PR consists of:

• Remove the Joomla-specific parts of CRM_Core_Error::handleUnhandledExecption()
• Remove if config->userFramework != 'Joomla' from templates/CRM/common/fatal.tpl so that a full html page is output
• Remove CRM_Utils_System_Joomla::outputError() altogether so that the parent CRM_Utils_System::outputError() is used instead.

Comments

Joomla's JError is deprecated so this also helps remove instances of its usage.

[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]

test this please

[eileenmcnaughton]

@vingle are you able to help with review on all this great Joomla! work @andrewpthompson is doing?

[vingle]

@eileenmcnaughton - I’m on holiday this week but will test when I’m back after Monday (any tips on a simple way to trigger an error message?)

[eileenmcnaughton]

I think you get an error if you try to view a contact with an invalid id

[andrewpthompson]

@vingle To trigger errors I was using a contribution page with no payment processor defined and also trying to run the database upgrader when the db has already been upgraded. administrator/index.php?option=com_civicrm&task=civicrm/upgrade&reset=1
Thanks for any time you're able to spend on this - will be great to get some of the longstanding J issues fixed.

[vingle]

That works for me.

One thing perhaps worth noting is that handling of those errors front and backend is currently different. Ie /administrator/index.php?option=com_civicrm using the Joomla admin theme breaks with Joomla's error handling as detailed in the issue, but front-end, ie /index.php?option=com_civicrm renders the error properly in the theme (or at least it did in an old YooTheme template I tested this with - there's no unrendered HTML).

With this patch, both front and backend use CiviCRM's error styling, so anyone currently using custom styled, front-end error pages that match their public-facing theme would, I guess, lose that with this commit. That's probably a pretty rare use-case, but figured it's worth mentioning.

[eileenmcnaughton]

@andrewpthompson Do you have any thoughts on that last comment?

[totten]

@vingle wrote:

anyone currently using custom styled, front-end error pages that match their public-facing theme would, I guess, lose that with this commit.

civicrm-core has a setting fatalErrorHandler which you can use to define a custom function for emitting an error. I wonder if it work as an off-ramp to:

• Put this old code-block in a separate function (e.g. CRM_Utils_System_Joomla::mapToJError or CRM_Core_Error::useJoomlaError)
• In the post-upgrade message on Joomla sites, advise that the error-handling has been normalized to consistently display error messages - but that you can continue using JError by setting the option fatalErrorHandler to CRM_Utils_System_Joomla::mapToJError (or whatever it's called)

(Of course, that would be have to be tested, and I don't know off the top of my head the best place to put that function..)

OTOH, if no one is likely to use the option, then just KISS and carry on without...

[andrewpthompson]

@vingle In every Joomla-provided template that I checked in J3 and J4 the error messages go through htmlspecialchars() so the unrendered html markup is displayed in frontend and backend. I didn't check any 3rd-party Joomla templates but I guess it's not too surprising that you've found one that doesn't do this.

I'm personally not too concerned about simply using CiviCRM's styling as these fatal errors or unhandled exceptions are the nasty things that shouldn't normally happen. I like @totten's suggestion though and could have a look at this is Joomla folk think it would be of use.

[vingle]

I'm personally not too concerned about simply using CiviCRM's styling as these fatal errors or unhandled exceptions are the nasty things that shouldn't normally happen.

@andrewpthompson I agree – I don't think this needs to be mitigated as Civi errors shouldn't be visible to end-users unless the site is broken. I just thought I should mention/log in case someone wondered why their error messages had changed.

[eileenmcnaughton]

OK - since you are in agreement I will merge - perhaps @agh1 can highlight in the release notes