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

Correctly add the preview script #4857

Merged
merged 3 commits into from Jun 23, 2022
Merged

Conversation

leofeyer
Copy link
Member

If Contao runs in a subfolder (which we allow locally), the /preview.php fragment will be added incorrectly.

URL: https://domain.wip/managed-edition/public/contao/preview
Before: https://domain.wip/preview.php/managed-edition/public/contao/preview
After: https://domain.wip/managed-edition/public/preview.php/contao/preview

@leofeyer leofeyer self-assigned this Jun 22, 2022
@github-actions github-actions bot added the bug label Jun 22, 2022
@leofeyer leofeyer requested a review from a team June 22, 2022 11:03
@leofeyer leofeyer added this to the 4.13 milestone Jun 22, 2022
Copy link
Member

@aschempp aschempp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about all the other places, like PageModell::getPreviewUrl()?

also, should it ever be possible to set a preview script that includes a subfolder/path? Or maybe we should add a validation for that?

also, this seems to assume the default entry point is never called with a script, or am I reading this wrong?

@leofeyer
Copy link
Member Author

what about all the other places, like PageModell::getPreviewUrl()?

They were not affected in my tests, probably because we are using the UrlGenerator with a custom context there instead of the request object.

@leofeyer leofeyer merged commit 13bfd10 into contao:4.13 Jun 23, 2022
@leofeyer leofeyer deleted the fix/preview-redirect branch June 23, 2022 07:39
@qzminski
Copy link
Member

qzminski commented Jun 28, 2022

It look like that after this commit, the frontend preview URLs like https://domain.tld/contao/preview?page=6 no longer work. What happens is that the user is redirected to the https://domain.tld/preview.php/contao/preview (notice missing query parameters) and the frontend preview always shows the homepage.

I think the fix would be:

-return new RedirectResponse($request->getBasePath().$this->previewScript.$request->getPathInfo());
+return new RedirectResponse($request->getBasePath().$this->previewScript.$request->getPathInfo().($request->getQueryString() ? ('?' . $request->getQueryString()) : ''));

But I am not completely sure.

@Toflar
Copy link
Member

Toflar commented Jun 28, 2022

Looks correct to me. Would you be so kind to create a PR? I think to show the problem it should be sufficient to add a query to any of the tests ;)

@qzminski
Copy link
Member

See #4891

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

Successfully merging this pull request may close these issues.

None yet

5 participants