[#18429] - Modern routing - remove view from query string
- Fixed in Code Base
- 7 Dec 2017
- Medium
- Build: staging
- # 18429
- Diff
- csthomas:routing_build_with_layout
User tests: Successful: Unsuccessful:
Summary of Changes
Build a correct URL for links without layout or when the layout from URL is different than from menu item.
Testing Instructions
-
Install staging joomla or 3.8.2 with sample data.
-
On back-end enable modern routing in
users. -
Optional enable
Allow User Registration -
Go to home page and:
- check module login links,
- try to login with wrong password and check URL address after that
- check URL address on profile edit form
Before After /index.php/password-reset?view=reset/index.php/password-reset/index.php/username-reminder?view=remind/index.php/username-reminder/index.php/login?view=login/index.php/login/index.php/registration-form?view=registration/index.php/registration-form/index.php/your-profile?view=profile&layout=edit/index.php/your-profile?layout=edit
Expected result
Links with own menu item does not have a view parameter in query string.
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries Unit Tests |
| Title |
|
||||||
| Labels |
Added:
?
?
|
||
If I understand correctly, the additional changes you added to this PR is only needed if someone pass layout=default in JRoute::_ call (which is wrong, I think).
The original code looks fine to me. We are working on a PR for staging, so I would stay on safe side, the less changes, the better, only the necessary change to address the issue should be implemented.
I have tested this item
Works as described, done a code review, too. I am not happy with some changes in the code, but It might be personal coding style only
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18429.
I reverted the code with non strict comparison to the false.
@joomdonation can you please retest?
@franz-wohlkoenig Will do after work. Maybe you can test it as well, just follow the instructions or ask us if needed. We need one more test anyway
Cannot test next Days as i'm not at Home.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18429.
| Title |
|
||||||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18429.
| Status | Pending | ⇒ | Ready to Commit |
Ready to Commit after two successful tests.
mbabker
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-12-07 13:00:24 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
So I looked at the code changes and also tested, it is working fine and solve the mentioned issue. However, there are some changes in the code which I am unsure if it is necessary, Maybe less change like I did here will still do the same thing https://github.com/joomla/joomla-cms/compare/staging...joomdonation:routing-fix-simpler?expand=1