Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent double amp encoding #16089

Merged
merged 1 commit into from May 17, 2017

Conversation

Projects
None yet
@PhilETaylor
Copy link
Contributor

commented May 17, 2017

Pull Request for Issue #16088 .

Summary of Changes

fix the double amp issue

Steps to reproduce the issue

Updated from 3.7.0. to 3.7.1
Turn SEF off in Joomla config.
Create menu items to category blog

Expected result

Something like
index.php?option=com_content&view=categories&id=50&Itemid=288

Actual result

double ampsands

index.php?option=com_content&view=categories&id=50& amp;Itemid=288

@ot2sen

This comment has been minimized.

Copy link
Contributor

commented May 17, 2017

I have tested this item successfully on 7928a9b

After applying patch the URL turn back to expected like index.php?option=com_content&view=article&id=8&Itemid=116


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16089.

@pskarsjo

This comment has been minimized.

Copy link

commented May 17, 2017

The double ampersands make the menu items behave unexpectedly. Sidebar modules are not displayed when menu item is called with the double ampersands.
Workaround: move the menu items to a non displaying menu. Create new menu items, type external link, with manually constructed link not containing the double ampersand.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16089.

@PhilETaylor

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2017

The double ampersands make the menu items behave unexpectedly. Sidebar modules are not displayed when menu item is called with the double ampersands.

Yes this I know, and this is fixed with this PR

The workaround IS THIS PR - you're meant to be testing it, not commenting on what the original issue is and a scary way of avoiding it.

@Quy

This comment has been minimized.

Copy link
Contributor

commented May 17, 2017

I have tested this item successfully on 7928a9b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16089.

@nordmograph

This comment has been minimized.

Copy link

commented May 17, 2017

I have tested this item successfully on 7928a9b

@pskarsjo

This comment has been minimized.

Copy link

commented May 17, 2017

Sorry. New guy.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16089.

@PhilETaylor

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2017

RTC anyone?

@dgrammatiko

This comment has been minimized.

Copy link
Contributor

commented May 17, 2017

RTC

@joomla-cms-bot joomla-cms-bot added the RTC label May 17, 2017

@nigelbpeck

This comment has been minimized.

Copy link

commented May 17, 2017

What does RTC mean please?

@zero-24

This comment has been minimized.

Copy link
Contributor

commented May 17, 2017

What does RTC mean please?

Ready to Commit. 😄

@nigelbpeck

This comment has been minimized.

Copy link

commented May 17, 2017

Ready to Commit. 😄

Ah ha! Cool. Thanks. Thought it might have been that, but it seemed more like that should have been ready to merge.

@PhilETaylor

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2017

@rdeutz Joomla 3.7.2 please

@wilsonge wilsonge merged commit cfd2480 into joomla:staging May 17, 2017

4 checks passed

JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot added PR-staging and removed RTC labels May 17, 2017

@wilsonge wilsonge added this to the Joomla 3.7.2 milestone May 17, 2017

@wojsmol

This comment has been minimized.

Copy link
Contributor

commented May 19, 2017

@PhilETaylor Can this issue couse generating backend links as SEF links?

@PhilETaylor

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2017

@wojsmol No I dont believe it could.

@wojsmol

This comment has been minimized.

Copy link
Contributor

commented May 19, 2017

Thanks @PhilETaylor then I have wird issue to debug.

@PhilETaylor

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2017

@wojsmol Please open a new issue with full details

@PhilETaylor PhilETaylor deleted the PhilETaylor:doubleamp branch May 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.