bug PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar Lab5-Switzerland
Lab5-Switzerland
26 Apr 2023

Proper checking of isset conditions, roborating the code. Otherwise, in case there is no form set, code will crash ( with error : "Call to a member function getXml() on null" ). Please Update the code accordingly.

Pull Request for Issue # .

Summary of Changes

Adds isset check to prevent crash of code, when improperly assumed condition ( ->form exists ) is not met. Roborates the Code.

Testing Instructions


use Joomla\CMS\Form\Field\TextField;

$feld = new TextField();
$xmlstring = '
<field
		name="mytest"
		label="My Test Standalone Field"
		type="test" 
/> 
';
$xml = new SimpleXMLElement( $xmlstring );
$feld->setup( $xml );
$field =  $feld->renderField();

As you will see, the Application crashes, simply because of the afore mentioned inpropper check, that falsely and blindly assumes, that there is a contextual form assosiated with the field ( which is never the case, when you set up a 'standalone' field ).
The source of the crash can be found here :

/libraries/src/Form/FormField.php line 1018+ :

        $options['inlineHelp'] = isset($this->form->getXml()->config->inlinehelp['button'])
            ? ((string) $this->form->getXml()->config->inlinehelp['button'] == 'show' ?: false)
            : false;

Again, the inaccuracy of the code here does not even have to to with Joomla per se, it's simply good coding practice to NEVER blindly assume anything at all, but to make sure. But of course, in this specific case, not fixing it would mean the completely unneccesary abandonment of a whole feature in Joomla! : The ability to have Joomla! FormFields set up in a 'standalone' manner, which are a beautiful feature, since they profit from all the beautiful typical Joomla perks in regards to FormField's features, like e.g. skinnability ( layouts ), etc.

Actual result BEFORE applying this Pull Request

application crash with error : "Call to a member function getXml() on null"

Expected result AFTER applying this Pull Request

just works even when there is no form present / set, aka when you set up a 'standalone' Field as exemplified in the above code.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 26 Apr 2023
Category Libraries
avatar Lab5-Switzerland Lab5-Switzerland - open - 26 Apr 2023
avatar Lab5-Switzerland Lab5-Switzerland - change - 26 Apr 2023
Status New Pending
avatar Lab5-Switzerland
Lab5-Switzerland - comment - 26 Apr 2023

Additional Info / Context :
I found this weakness, when trying to setup and subsequently render a field 'standalone', independent of any form.
In my case i needed a standaonone selectbox field on a page. Setting it up was no problem, but when attempting to render via $field->renderField(); , i stumbled over the issue described in this pull request.

avatar Lab5-Switzerland Lab5-Switzerland - change - 29 Apr 2023
Title
Update FormField.php - Code roboration / Bugfix
Update to FormField.php - Code roboration / Bugfix ( 1 minute fix )
avatar Lab5-Switzerland Lab5-Switzerland - edited - 29 Apr 2023
avatar laoneo
laoneo - comment - 5 May 2023

Thanks for the pr. Can you add proper testing instructions? Like that people can test it and maintainers do understand where the issue is coming from.

avatar Lab5-Switzerland
Lab5-Switzerland - comment - 6 Jun 2023

Hi guys.
( Sorry i didn't have much time lately, work shedule is insane. )

So here is a very basic and simple example code, so you can test this for youself, easily, quickly and comfortably:


use Joomla\CMS\Form\Field\TextField;

$feld = new TextField();
$xmlstring = '
<field
		name="mytest"
		label="My Test Standalone Field"
		type="test" 
/> 
';
$xml = new SimpleXMLElement( $xmlstring );
$feld->setup( $xml );
$field =  $feld->renderField();

As you will see, the Application crashes, simply because of the afore mentioned inpropper check, that falsely and blindly assumes, that there is a contextual form assosiated with the field ( which is never the case, when you set up a 'standalone' field ) :


/libraries/src/Form/FormField.php line 1018+ :

        $options['inlineHelp'] = isset($this->form->getXml()->config->inlinehelp['button'])
            ? ((string) $this->form->getXml()->config->inlinehelp['button'] == 'show' ?: false)
            : false;

So you either implement this very simple and obvious fix, OR you decide completely to abandon any posibility to ever set up fields in this standalone manner via Joomla ever again, which would be a a complete waste, because standalone filelds are very useful and do profit from all the beautiful typical Joomla perks in regards to field's features, like e.g. skinnability ( layouts ), etc.

Again, the inaccuracy of the code here does not even have to to with Joomla per se, it's simply good coding practice to NEVER blindly assume anything at all, but to make sure. But of course, in this specific case, not fixing it would mean the completely unneccesary abandonment of a whole feature in Joomla!.
Do not abandon the ability to have standalone fields, please, for the love of Joomla!

avatar Lab5-Switzerland Lab5-Switzerland - change - 6 Jun 2023
The description was changed
avatar Lab5-Switzerland Lab5-Switzerland - edited - 6 Jun 2023
avatar Lab5-Switzerland Lab5-Switzerland - change - 6 Jun 2023
The description was changed
avatar Lab5-Switzerland Lab5-Switzerland - edited - 6 Jun 2023
avatar Lab5-Switzerland
Lab5-Switzerland - comment - 6 Jun 2023

Thanks for the pr. Can you add proper testing instructions? Like that people can test it and maintainers do understand where the issue is coming from.

Did now - I just updated the original post for the commit pull request.

avatar Lab5-Switzerland Lab5-Switzerland - change - 6 Jun 2023
Labels Added: bug PR-4.3-dev
avatar laoneo
laoneo - comment - 8 Jun 2023

Thanks, now we need some testers and then it can be merged.

avatar Lab5-Switzerland
Lab5-Switzerland - comment - 11 Jun 2023

Please update the priority; All Joomla! instances that use standalone fields are currently crashing on the respective pages.
Please do actually have a quick look at the code, and convience yorself by the mere fact of the nature of the change, that it is indeed, actually and literally a 1 minute fix and actually self-explanatory, even if you do not consider oder believe in the immense effects it has ( which is : Keeping the standalone fields feature alive of completely killing it, if this absolutely supersimple fix is not implemented ).

Even if you do not test it, the change is save and self-explanatory. Just have superquick look at the code, i',m begging you.

( I case i might sound impatient : I actually am, lol. It cannot be, that i have to update and fix the Joomla! core of several sites every time there's an update, just because of something brainlessly easily fixable like this. )

avatar laoneo
laoneo - comment - 20 Jun 2023

If possible, it would be nice to add a unit test. If we don't get any tests till the weekend. I'm going to merge this one as it indeed stabilizes the core code.

avatar laoneo laoneo - change - 20 Jun 2023
Labels Added: PR-4.4-dev
Removed: PR-4.3-dev
avatar laoneo laoneo - close - 26 Jun 2023
avatar laoneo laoneo - merge - 26 Jun 2023
avatar laoneo laoneo - change - 26 Jun 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-06-26 17:44:53
Closed_By laoneo
avatar laoneo
laoneo - comment - 26 Jun 2023

Thanks!!

Add a Comment

Login with GitHub to post a comment