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

Strip base_url only from beginning of relative url #15418

Merged
merged 1 commit into from
Apr 7, 2021
Merged

Strip base_url only from beginning of relative url #15418

merged 1 commit into from
Apr 7, 2021

Conversation

Jako
Copy link
Collaborator

@Jako Jako commented Feb 26, 2021

What does it do?

Use a regular expression to replace MODx.config.base_url only at the beginning of the submitted relative url.

Why is it needed?

MODx.config.base_url should only occur at the beginning of the beginning of the submitted relative url. Currently it is stripped everywhere in the string.

How to test

Set the value of the base_url system setting to i.e. '/test/' and create/edit a static resource. Browse for the static resource file and edit the path at the bottom manually afterwards and insert the string '/test/' somewhere in that path. Before it will be replaced everywhere, afterwards it is only replaced at the start of the string.

Related issue(s)/PR(s)

None known.

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Feb 26, 2021
@Ibochkarev Ibochkarev added this to the v2.8.2 milestone Feb 27, 2021
@Ibochkarev Ibochkarev added the pr/review-needed Pull request requires review and testing. label Feb 27, 2021
@alroniks
Copy link
Collaborator

alroniks commented Mar 9, 2021

@Jako I can not reproduce it. I tried to find such a system setting, but it does not exist. Also, I tried changing it directly in config.inc.php, but also all works as expected. The same results with adjusting baseUrl in the media source.

@Jako
Copy link
Collaborator Author

Jako commented Mar 9, 2021

The setting is used i.e. in LangRouter/xRouting + Babel sites as context setting with a different value than /. And I my client had issues with not found static PDF files because some parts of the path were stripped away after the file was selected in the MODX browser.

@alroniks alroniks added the blocked Active participation around the pull request or issue required. Consensus is not reached. label Mar 9, 2021
@Jako
Copy link
Collaborator Author

Jako commented Mar 17, 2021

Why did you add that label?

@alroniks alroniks removed the blocked Active participation around the pull request or issue required. Consensus is not reached. label Mar 29, 2021
@opengeek opengeek changed the title Strip MODx.config.base_url only at the beginning of the submitted relative url Strip base_url only from beginning of relative url Apr 2, 2021
@opengeek
Copy link
Member

opengeek commented Apr 2, 2021

@Jako — does this issue apply to 3.x, as well?

@Jako
Copy link
Collaborator Author

Jako commented Apr 2, 2021

Maybe, the JavaScript can be the same.

@Jako
Copy link
Collaborator Author

Jako commented Apr 4, 2021

@Jako Jako added the pr/port-to-3 Pull request has to be ported to 3.x. label Apr 4, 2021
@opengeek opengeek removed the pr/review-needed Pull request requires review and testing. label Apr 7, 2021
@opengeek opengeek merged commit 193b44a into modxcms:2.x Apr 7, 2021
@Jako Jako deleted the fix_strip_base_url branch May 15, 2021 04:27
@Ruslan-Aleev Ruslan-Aleev removed the pr/port-to-3 Pull request has to be ported to 3.x. label Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants