[#15376] - [Isis] Use getInt over get as we just accept integers here
- Fixed in Code Base
- 11 Jul 2017
- Medium
- Build: staging
- # 15376
- Diff
- zero-24:cast_int
- Success hound No violations found. Woof! Details
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
User tests: Successful: Unsuccessful:
Summary of Changes
Use getInt over get as we just accept integers here
Testing Instructions
- review
- check that using isis still work
Expected result
Isis still work
Actual result
we not cast to int
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Templates (admin) |
It's a valid PR.
The current $input->get('Itemid', ''); is wrong as there is only the default filtering.
We should either use $input->getInt('Itemid', 0); or $input->get('Itemid', 0, 'int'); since we only accept integers here.
I'm going to update this PR in a minute.
| Labels |
Added:
?
|
||
Before:
<body class="admin com_content view- layout- task- itemid-" data-basepath="/joomla-cms-staging">
After:
<body class="admin com_content view- layout- task- itemid-0" data-basepath="/joomla-cms-staging">
The difference is itemid- vs itemid-0. I am wondering if Itemid should be id.
Itemid and id in the context of Joomla are two different things.
Here is a URL:
http://localhost/joomla-cms-staging/administrator/index.php?option=com_content&view=article&layout=edit&id=1
Here is the code to get values in the URL. As you can see it is id so Itemid will always be 0.
// Detecting Active Variables
$option = $input->get('option', '');
$view = $input->get('view', '');
$layout = $input->get('layout', '');
$task = $input->get('task', '');
$itemid = $input->get('Itemid', '');
Itemid doesn't actually apply in the backend context at all (unless we're suddenly routing to menu items in the backend) so I'm honestly not sure why it's used in the template. But for the frontend they are different things.
So deprecate it, change to id or leave it as is?
This PR as is is fine. I'd suggest the 4.0 admin template doesn't need an Itemid based HTML class though unless we're opening the can of worms needed to support SEF routing for the admin app.
In error.php, $itemid is declared but not utilized.
Any update here so we can move this forward to be included in the 3.7.3?
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15376.
brianteeman
- | Milestone |
Added: |
||
| Milestone |
Added: |
||
| Milestone |
Removed: |
||
| Milestone |
Removed: |
||
| Milestone |
Added: |
||
I have tested this item
Code review and tested, Itemid is now returned as 0 after applying patch.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15376.
| Status | Pending | ⇒ | Ready to Commit |
RTC as stated above.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-11 14:14:35 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
|
||
Would this be closed in light of this PR #15448?