User tests: Successful: Unsuccessful:
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.
Thanks for the tip.
However I'm looking for some garbage cleanup. Those commits are still there but hidden.
Labels |
Removed:
?
|
Title |
|
Status | New | ⇒ | Pending |
Title |
|
Category | ⇒ | Libraries |
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!
Status | Pending | ⇒ | Information Required |
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).
Yes, it should be updated. Even though something is deprecated, it should continue to function correctly until it is removed.
@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.
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>';
Tested it but I couldn't see any differences between the changes.
So I couldn't reproduce it.
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 . '"]'))
Tested with Joomla version: 3.4.4-dev development
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!
Status | Information Required | ⇒ | Ready to Commit |
Just tested this successful with the sample code by @izharaazmi. Based on the test by @kathastaden I'm RTC'ing here. Thanks!
Labels |
Added:
?
|
Milestone |
Added: |
Milestone |
Added: |
Milestone |
Removed: |
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-09-08 23:47:16 |
Closed_By | ⇒ | roland-d |
Labels |
Removed:
?
|
Milestone |
Milestone |
Added: |
Milestone |
Added: |
Milestone |
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.
Hi @izharaazmi, can you please do a PR against staging with your commit (izharaazmi@64bc802)? Thanks!
@Kubik-Rubik It's on the way!
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.