[#33834] Fixed JForm::bind for nested JObject argument

[izharaazmi]

JForm::bind method was relying on getProperties() which returned only the attributes of the given object. However in case one of these attributes happens to be a JObject instance itself, then JForm::bindLevel would fail there.
In particular model's getItem function tends to return a nested JObject always.

JArrayHelper call would take care of JObject to Array conversion recursively.

http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33834&start=0

[Achal-Aggarwal]

You can use git squash to make a single commit from these 6 commits. More about it is at http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

For current case
git rebase -i HEAD~6 and then put "pick" in front of commit ccb2d74 and "squash" in front of others.

[izharaazmi]

Thanks for the tip. 👍
However I'm looking for some garbage cleanup. Those commits are still there but hidden.

[roland-d]

Hello @izharaazmi

Thank you for your contribution.

The last comment here was on 8th June 2014. So the question is, is this issue/pull request still valid?
if so please provide clear test instructions to be able to test / reproduce this issue.
If no reply is received within 4 weeks we will close this issue.

Thanks for understanding!

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3737.}

[izharaazmi]

I dropped this idea as the JObject got deprecated. However the issue is
still valid as long the JObject is being used in Model's getItem() method.

Should I update the PR, or just close it?
On Aug 21, 2015 12:38 AM, "RolandD" notifications@github.com wrote:

Hello @izharaazmi https://github.com/izharaazmi

Thank you for your contribution.

The last comment here was on 8th June 2014. So the question is, is this
issue/pull request still valid?
if so please provide clear test instructions to be able to test /
reproduce this issue.
If no reply is received within 4 weeks we will close this issue.

Thanks for understanding!

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/3737
http://issues.joomla.org/tracker/joomla-cms/3737.

—
Reply to this email directly or view it on GitHub
#3737 (comment).

[mbabker]

Yes, it should be updated. Even though something is deprecated, it should continue to function correctly until it is removed.

[roland-d]

@izharaazmi If you can provide the test instructions soon that would be appreciated, I have a group of people testing issues tomorrow and I would be able to include this one. Thanks.

[izharaazmi]

To reproduce this we'd need the recursive JObject returned by the JModelAdmin being passed to JForm::bind() directly.

For quick test do the following:

Create a form XML with a field with xpath something like /fields[@name="level1"]/fields [@name="level2"]/field[@name="field1"]

Motive is to make the field deep nested.

Pass the JForm::bind a recursive JObject created from following object:

{level1: {level2: {field1: field_value}}}

Binding level1 would be fine, but level2 will cause error as the protected property $_error of JObject is included in the converted array in the bindLevel method of JForm which is preceded by a null byte.

This holds true for all subsequent levels.

My PR fixes this JObject to array conversion by using ArrayHelper instead of using getProperties method.

Sample code for test:

$form = JForm::getInstance('test', '<form><fields name="level1"><fields name="level2"><field name="field1" type="text"/></fields></fields></form>');
$array = array('level1' => array('level2' => array('field1' => 'field_value')));
$data = JArrayHelper::toObject($array, 'JObject');
$form->bind($data);

echo '<pre>';
print_r($data);
print_r($form);
echo '</pre>';

[mflr26]

Tested it but I couldn't see any differences between the changes.
So I couldn't reproduce it.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3737.}

[izharaazmi]

Make sure you have set error_reporting = ON

I just verified and it shows the following warnings without this patch. After this patch the warnings are gone.

Warning: SimpleXMLElement::xpath(): Unfinished literal in /Users/izharaazmi/Sites/joomla/libraries/joomla/form/form.php on line 1526
Warning: SimpleXMLElement::xpath(): xmlXPathEval: evaluation failed in /Users/izharaazmi/Sites/joomla/libraries/joomla/form/form.php on line 1526
Warning: SimpleXMLElement::xpath(): Unfinished literal in /Users/izharaazmi/Sites/joomla/libraries/joomla/form/form.php on line 1526
Warning: SimpleXMLElement::xpath(): xmlXPathEval: evaluation failed in /Users/izharaazmi/Sites/joomla/libraries/joomla/form/form.php on line 1526

The warning occurs at following line:

if ($tmp = $element->xpath('descendant::field[@name="' . $name . '"]'))

[kathastaden]

Tested with Joomla version: 3.4.4-dev development

• error_reporting enabled
• monitored the php_errors.log

1. Created a little component which contained the example code of @izharaazmi
2. Patch was not applied yet: I got the same warnings as @izharaazmi
3. Patch was applied: The warnings were no longer shown and nothing was written into the php_errors.log anymore

Test was successful!

[zero-24]

Just tested this successful with the sample code by @izharaazmi. Based on the test by @kathastaden I'm RTC'ing here. Thanks!

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3737.}

[vietvh]

Hi,

Thank you for all of the work on Joomla 3.5 so far!

I've just tested our code on Joomla 3.5 Beta and something didn't work well, then I did a back tracking and found the root cause is the change in this pull request.

In our case, our Model return $item which has the following structure:

$item (JObject)
|-- $foo (Array)
|-- |-- 0 => (stdClass)
|-- |-- 1 => (stdClass)
|-- |-- 2 => (stdClass)

This structure has been worked fine since Joomla 3.0.0 and stopped this morning because JForm will automatically convert all nested objects to array silently.

We can make a work around this but I am afraid of how many extensions out there are relying on the old JForm behavior.

[Kubik-Rubik]

Hi @izharaazmi, can you please do a PR against staging with your commit (izharaazmi@64bc802)? Thanks!

[izharaazmi]

@Kubik-Rubik It's on the way!