? Success

User tests: Successful: Unsuccessful:

avatar izharaazmi
izharaazmi
7 Jun 2014

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

avatar izharaazmi izharaazmi - open - 7 Jun 2014
avatar Achal-Aggarwal
Achal-Aggarwal - comment - 7 Jun 2014

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.

avatar izharaazmi
izharaazmi - comment - 8 Jun 2014

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

avatar nicksavov nicksavov - change - 21 Aug 2014
Labels Removed: ?
avatar nicksavov nicksavov - change - 21 Aug 2014
Title
Fixed JForm::bind for nested JObject argument
[#33834] Fixed JForm::bind for nested JObject argument
avatar brianteeman brianteeman - change - 21 Aug 2014
Status New Pending
avatar izharaazmi izharaazmi - change - 21 Aug 2014
Title
Fixed JForm::bind for nested JObject argument
[#33834] Fixed JForm::bind for nested JObject argument
avatar brianteeman brianteeman - change - 2 Sep 2014
Category Libraries
avatar roland-d
roland-d - comment - 20 Aug 2015

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.

avatar roland-d roland-d - change - 20 Aug 2015
Status Pending Information Required
avatar izharaazmi
izharaazmi - comment - 20 Aug 2015

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).

avatar mbabker
mbabker - comment - 20 Aug 2015

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

avatar roland-d
roland-d - comment - 20 Aug 2015

@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.

avatar izharaazmi
izharaazmi - comment - 20 Aug 2015

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>';
avatar mflr26
mflr26 - comment - 21 Aug 2015

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.

avatar mflr26 mflr26 - test_item - 21 Aug 2015 - Not tested
avatar izharaazmi
izharaazmi - comment - 21 Aug 2015

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 . '"]'))
avatar kathastaden kathastaden - test_item - 21 Aug 2015 - Tested successfully
avatar kathastaden
kathastaden - comment - 21 Aug 2015

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!

avatar zero-24 zero-24 - change - 23 Aug 2015
Status Information Required Ready to Commit
avatar zero-24
zero-24 - comment - 23 Aug 2015

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.

avatar joomla-cms-bot joomla-cms-bot - change - 23 Aug 2015
Labels Added: ?
avatar zero-24 zero-24 - test_item - 23 Aug 2015 - Tested successfully
avatar zero-24 zero-24 - change - 25 Aug 2015
Milestone Added:
avatar Kubik-Rubik Kubik-Rubik - change - 3 Sep 2015
Milestone Added:
avatar Kubik-Rubik Kubik-Rubik - change - 3 Sep 2015
Milestone Removed:
avatar roland-d roland-d - reference | 9f05d19 - 8 Sep 15
avatar roland-d roland-d - merge - 8 Sep 2015
avatar roland-d roland-d - close - 8 Sep 2015
avatar roland-d roland-d - change - 8 Sep 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-09-08 23:47:16
Closed_By roland-d
avatar roland-d roland-d - close - 8 Sep 2015
avatar joomla-cms-bot joomla-cms-bot - close - 8 Sep 2015
avatar joomla-cms-bot joomla-cms-bot - change - 8 Sep 2015
Labels Removed: ?
avatar izharaazmi izharaazmi - head_ref_deleted - 9 Sep 2015
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone
avatar vietvh
vietvh - comment - 11 Nov 2015

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.

avatar izharaazmi izharaazmi - reference | 64bc802 - 11 Nov 15
avatar Kubik-Rubik
Kubik-Rubik - comment - 11 Nov 2015

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

avatar izharaazmi
izharaazmi - comment - 11 Nov 2015

@Kubik-Rubik It's on the way!

Add a Comment

Login with GitHub to post a comment