? Release Blocker PR-5.0-dev Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
9 Aug 2023

Pull Request for Issue #41256 .

Summary of Changes

After removing CMSObject from User class #40999, it cause weird issue when binding User data to the form.
The PR is fixing it.

Testing Instructions

Apply patch,
Make sure errror reaporting on maximum.
On the site open user profile page

Actual result BEFORE applying this Pull Request

No warnings

Expected result AFTER applying this Pull Request

Many warnings:

Warning: SimpleXMLElement::xpath(): Unfinished literal in /libraries/src/Form/Form.php on line 1250

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 - 9 Aug 2023
Category Libraries
avatar Fedik Fedik - open - 9 Aug 2023
avatar Fedik Fedik - change - 9 Aug 2023
Status New Pending
avatar waader waader - test_item - 26 Aug 2023 - Tested successfully
avatar waader
waader - comment - 26 Aug 2023

I have tested this item ✅ successfully on a22f55b

works for me!


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

avatar Quy Quy - test_item - 2 Sep 2023 - Tested successfully
avatar Quy
Quy - comment - 2 Sep 2023

I have tested this item ✅ successfully on a22f55b


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

avatar Quy Quy - change - 2 Sep 2023
Status Pending Ready to Commit
Labels Added: PR-5.0-dev
avatar Quy Quy - edited - 2 Sep 2023
avatar Quy
Quy - comment - 2 Sep 2023

RTC


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

avatar HLeithner
HLeithner - comment - 2 Sep 2023

Not sure if this is the right solution...

avatar HLeithner HLeithner - close - 3 Sep 2023
avatar HLeithner HLeithner - merge - 3 Sep 2023
avatar HLeithner HLeithner - change - 3 Sep 2023
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-09-03 08:24:16
Closed_By HLeithner
Labels Added: ? Release Blocker
avatar HLeithner
HLeithner - comment - 3 Sep 2023

I merge this even if I'm not 100% confident with this solution. thanks

avatar HLeithner
HLeithner - comment - 3 Sep 2023

maybe the alternative is to check if the legacytrait is implemented.

avatar Fedik
Fedik - comment - 3 Sep 2023

Maybe we can implement Iterator intrface over base Table class?

avatar HLeithner
HLeithner - comment - 3 Sep 2023

hmm that's maybe an idea too

avatar Fedik
Fedik - comment - 3 Sep 2023

then it also will be possible to do $array = iterator_to_array($table) https://www.php.net/manual/en/function.iterator-to-array

avatar korenevskiy
korenevskiy - comment - 4 Dec 2024

@Fedik
The same problem also arose.
but this fix didn't help.
Error in the Form class in the findField() method.
There is code in this method
$fields = $this->xml->xpath('//field[@name="' . $name . '" and not(ancestor::field/form/*)]');
which causes the error specified in the subject of this PR.
The error pops up because $name contains the value *_errors
I spent half a day looking for where this value falls out in $name. Found.
I had this happen when saving an article in one of the plugins.
Using the Content model.
Of the ArticleModel class in the loadFormData() method, the method is called
$this->preprocessData('com_content.article', $data); from trait FormBehaviorTrait
Method preprocessData($context, &$data, $group = 'content')' in 'trait FormBehaviorTrait contains the code

Factory::getApplication()->triggerEvent('onContentPrepareData', [$context, &$data]);

So in my case, initially $data had the object type, but after calling the trigger, the type became array.
One of the plugins converted the type from an object to an array and returned a link.
There should be no problems, because in the Form class, the bindLevel method takes into account that the binding occurs of an array or object.
But the fact is that the object had hidden properties.
In the case when the plugin has applied a function to the object

settype($data, 'array');

the hidden error field has turned into *_errors
and at the time of binding, this field causes an error when calling the function
$this->xml->xpath(); in the name of $name when it contains *_errors
I suggest in the Form->bindLevel($group, $data) method
, where the foreach($data as $k => $v)
iteration occurs, to check for the presence of an asterisk

then ignore the binding for the field with an asterisk

avatar Fedik
Fedik - comment - 4 Dec 2024

In the case when the plugin has applied a function to the object settype($data, 'array');

That is illegal, and it will not work in the future.

avatar korenevskiy
korenevskiy - comment - 6 Dec 2024

I agree with You that this is illegal.
And I hope in the future Joomla will tell us which plugin changes the object to an array or vice versa, thanks to the type check. But you must agree that it is difficult to intercept an error when the type changes somewhere in the plugin, and the error climbs into another from the opposite part of joomla.
But there is also another problem, the FORM class allows it to be bound as an array and as an object. Both methods are acceptable. But let's assess the situation, should the binding of the form give an error due to the fact that the name begins with an asterisk? or should such a name just be skipped?
After all, the binding code should just look for fields of the same name, if the field name does not match, then it does not match anyway. This may be intended by the algorithm of a plugin or component, it should not cause an error.
The problem appeared to me as a result of creating a module for saving articles, i.e. this error was not in the module that I was making, but in the plugin.
Maybe we need to wrap additional quotes around the name here?

$el->xpath('descendant::field[@name="' . $name . '" and not(ancestor::field/form/*)]')

Another question for you, Is the data transferred to the CONTENT plugin only in the form of an object? or can they be passed as an array?

avatar Fedik
Fedik - comment - 6 Dec 2024

But you must agree that it is difficult to intercept an error when the type changes somewhere in the plugin

Should write to developer of the plugin, that it is a bug in their plugin.

or can they be passed as an array?

Currently can be both, depend from Model and Request. But it should not be changed at the middle of runtime.

avatar korenevskiy
korenevskiy - comment - 6 Dec 2024

Should write to developer of the plugin, that it is a bug in their plugin.

And how do you recommend to access the data?
After all, the data can be either an object or an array.
And if you refer to the value of the array as the value of the object, there will be an error. That's why I understand the plugin developer. that he made the type conversion to an array so that there would be no problems that he was accessing the value in the wrong format.

avatar Fedik
Fedik - comment - 7 Dec 2024

Whe you realy need, you can do:

$myData = (array) $data;

But do not:

$data = (array) $data;
avatar Fedik
Fedik - comment - 7 Dec 2024

When you realy need, you can do:

$myData = (array) $data;

But do not:

$data = (array) $data;
avatar korenevskiy
korenevskiy - comment - 7 Dec 2024

But if you need to change the article data inside the plugin. Then we have to use 2 ways. 1.Modify as an object, and 2.Modify as an array. And before that, check the type.

Add a Comment

Login with GitHub to post a comment