[#10876] - 2 issues in custom forms that have ACL rules: AJAX ACL saving can break, also form HTML has hundreds HTML validation errors
- Closed
- 22 Jun 2016
- Medium
- Build: master
- # 10876
Steps to reproduce the issue
Open a custom form that does not have "title" form field and try to change ACL rules
To replicate issue without a such a custom form:
Open any Joomla form that has ACL rules
Find the "title" field and edit DOM to delete (e.g. use firebug)
Then try to change an ACL rule
Expected result
An AJAX request is made and the ACL rule is saved and the form updates to show the result
Actual result
Before the AJAX call is made we get a JS error
TypeError: document.getElementById(...) is null
title = document.getElementById('jform_title').value;
permissions-uncompressed.js (γραμμή 28, στήλη 11)
or
TypeError: document.getElementById(...) is null
title = document.getElementById('jform_title').value;
permissions-uncompressed.js (γραμμή 32, στήλη 11)
System information (as much as possible)
Current staging
Additional comments
Enabled Joomla Debug (so that you can see the JS error in permissions-uncompressed.js)
The issue does not exist in J3.5.1, i think (not sure) it was introduced by: #10793
Also i get hundreds or thousands of HTML validation errors because of this line (mentioned below) is not escaping the contents of the "title" TAG parameter (the more usergroups you have and the more ACL rules with titles that have special letters that need to be encoded)
Line is:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/fields/rules.php#L288
. ' title="' . JText::sprintf('JLIB_RULES_SELECT_ALLOW_DENY_GROUP', JText::_($action->title), trim($group->text)) . '">';
should escape its contents !!! like
. ' title="' . htmlspecialchars(JText::sprintf('JLIB_RULES_SELECT_ALLOW_DENY_GROUP', JText::_($action->title), trim($group->text)), ENT_COMPAT, 'UTF-8') . '">';
| Title |
|
||||||
| Title |
|
||||||
@andrepereiradasilva
thanks for all your time and efforts,
i did not suggest the the code does not exist in J3.5.1
yes, code of both issues is there in J3.5.1
now,
about the first issue, the JS error is not triggered in J3.5.1
about the second issue, the validation errors, yes they are created in J3.5.1 too
only 1st issue may or may not be related to the PR i mentioned
well, what i know is that in the ACL PR i didn't change that part.
See:
@ggppdk i just tried your first issue in a clean 3.5.1 install
Conclusion: The first issue you describe also exists in 3.5.1
What i did:
- Clean 3.5.1 with test data
- Edit a category, changed the title field id by editing dom in the console
- Changed any permission: error
permissions.js:29 Uncaught TypeError: Cannot read property 'value' of null
For 1st issue i can work around in my extension with a hidden 'title' field for forms that need this
For 2nd issue, it is not important, but it is frustrating to see all this unneeded HTML validation errors, if it was 1 error and it was needed to have such HTML, then ok but it is something repeatable and unneeded
Anyway i think that fixes for both are obvious, easy and small
I can make a PR on wednesday or sooner, if there are 2 people here that are willing to test
i'm willing to test
Will do, thanks
brianteeman
- | Labels |
Added:
?
|
||
| Category | ⇒ | ACL |
| Title |
|
||||||
| Title |
|
||||||
About requirement to have form field:
id="jform_title"
i will not suggest a PR, instead i will patch my specific form, to have this as a hidden field
- about the HTML validation errors due to HTML inside the title parameter of the ACL selector (inherit / allow / deny selector)
<select title="..."
i have made a PR #10906
- that strips html tags
Unless some one decides that we are not allowed to have a language string with HTML as title, my PR should be considered and accepted ??
(if you tell me that HTML is not allowed there then i will remove the HTML from the language strings)
[EDIT]
Also the issue with jform_title is not new,
it happened in the specific form that lacks jform_title, when AJAX saving was first introduced, just i have not noticed before, because, it is happening on 2nd form save for me
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-06-22 19:54:50 |
| Closed_By | ⇒ | ggppdk |
@ggppdk
In 3.5.1 the
document.getElementById('jform_title')is already thereSee https://github.com/joomla/joomla-cms/blob/3.5.1/media/system/js/permissions.js#L29
In 3.5.1 the title is already there
See https://github.com/joomla/joomla-cms/blob/3.5.1/libraries/joomla/form/fields/rules.php#L280
So are you sure this doesn't also exist in 3.5.1?