[#6359] - Fix option value must not be case sensitive
- Closed
- 7 May 2016
- Medium
- Build: staging
- # 6359
- Diff
- itbra:patch-30
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
When trying to fetch component information like its parameters passing in the component name in uppercase format then that component won't be found and an error JLIB_APPLICATION_ERROR_COMPONENT_NOT_FOUND will be thrown, where as when passing the name in lower case format everything will works properly.
I think case sensitivity should play absolutely not a role at this important place to ensure the component information will be fetched no matter in what case the option value is formatted.
Thus i added to every related method in this class receiving the component name as a function parameter a case fix.
joomla-cms-bot
- | Labels |
Added:
?
|
||
@test
Joomla3.4, XAMPP 1.8.1
Tested successfully with
$test1 = JComponentHelper::getComponent('COM_CONTENT', true);
echo 'DEBUG: getComponent '.print_r($test1,true);
echo "\n
";
$test2 = JComponentHelper::isEnabled('COM_CONTENT');
echo 'DEBUG: isEnabled '.print_r($test2,true);
echo "\n
";
$test3 = JComponentHelper::isInstalled('COM_CONTENT');
echo 'DEBUG: isInstalled '.print_r($test3,true);
echo "\n
";
$test4 = JComponentHelper::getParams('COM_CONTENT');
echo 'DEBUG: getParams '.print_r($test4,true);
echo "\n
";
Before patch. Fatal error or "empty" return values.
After patch: Correct return values.
isInstalled() is true before and after because MySQL-WHERE works case insensitive.
Additional tests with Joomla3.4 and testing content. Several components.
| Category | ⇒ | Libraries |
| Title |
|
||||||
Hi! Thanks for contributing!
However I think that this is not necessary & too much.
If we are not testing for Uppercase, why not do a trim as well, to ensure that we don't have whitespace in the name? Or test for the presence of com_? The option name is always written in small letters -> in the url, in the folder name, in the database etc.
I would consider this a coding standard and not something that we should take care of in our code.
With that said - I've tested your patch and it works, but I'm against adding this into the code.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6359.
Setting to NEEDS REVIEW for a CMS maintainer to make a decision
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6359.
| Status | Pending | ⇒ | Needs Review |
yvesh
- 
Thanks for reporting this "issue", but it's well-known that component names are lower case throughout our codebase. I don't see any particular value in adding more code to cover someone not using that "standard". On that basis, I'm closing this issue.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6359.
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-07 09:38:24 |
| Closed_By | ⇒ | zero-24 |
Set to "closed" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/6359
Closing based on the comment by @chrisdavenport Thanks.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6359.
All my comments mean is that you could optimize your PR.