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

Fix incorrect MediaLibrary links when base path is not the domain root #4055

Merged

Conversation

adsa95
Copy link
Contributor

@adsa95 adsa95 commented Jan 12, 2019

This PR fixes #4039, a bug that was introduced in PR #3536.

Currently the app root/base path is added to the storagePath variable in MediaLibrary, which is then used to generate the links with the getPathUrl() function. Since PR #3536 this is then also passed through the Url::to()function to adhere to the link policy, however the base path is added again in this method and therefore we get duplicated base paths in the final URL (only noticeable if your base path is not the domain root, and not using S3 etc). This PR solves the issue by not adding the base path to storagePath in the MediaLibrary.

storagePath is a protected variable and not used anywhere else within the MediaLibrary so this should not cause any other problems, it would however affect anyone extending the MediaLibrary class and using this variable.

Unfortunately I have no experience using tests, and not the time needed to learn this within a reasonable timeframe for the need of this PR. Ping @w20k

@LukeTowers
Copy link
Contributor

@w20k could you put together a test case for this?

@w20k
Copy link
Contributor

w20k commented Jan 17, 2019

@adsa95, could you look at #4065, seems like there is one more issue with path duplication... Odd one! Thanks 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants