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

[4.x]: Page trigger is reset on CP requests #12598

Closed
bencroker opened this issue Jan 31, 2023 · 6 comments
Closed

[4.x]: Page trigger is reset on CP requests #12598

bencroker opened this issue Jan 31, 2023 · 6 comments

Comments

@bencroker
Copy link
Contributor

bencroker commented Jan 31, 2023

What happened?

The page trigger is reset on CP requests, meaning that plugins cannot check its actual value.

cms/src/web/Request.php

Lines 307 to 311 in 9b483fa

if ($this->_isCpRequest) {
// Force 'p' pageTrigger
// (all that really matters is that it doesn't have a trailing slash, but whatever.)
$this->generalConfig->pageTrigger = 'p';
}

This leads to an inaccurate value when plugins fetch it, as reported in the Blitz issue at putyourlightson/craft-blitz#469

// Returns `p` instead of `?p=`
$pageTrigger = Craft::$app->getConfig()->getGeneral()->getPageTrigger();

Would it be possible to simply ensure that the page trigger doesn't have a trailing slash, as the code comment suggests? Alternatively, could the check for a page trigger only happen for site requests?

Craft CMS version

4.3.6.1

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@brandonkelly
Copy link
Member

I don’t want to move away from the control panel forcing the page trigger to be p, however we can do that without overriding the pageTrigger config setting. I’ve submitted a PR for Craft 3 which does that (#12614). Let me know if that will work for your needs @bencroker!

@bencroker
Copy link
Contributor Author

That's perfect, thanks @brandonkelly!

@bencroker bencroker reopened this Feb 3, 2023
@bencroker
Copy link
Contributor Author

@brandonkelly I just noticed that this wasn't added to any of the the Craft 4 branches, did I miss it or is it pending review first?

@brandonkelly
Copy link
Member

Just wanted to make sure it was going to work for you first. The change is now merged in for the next Craft 3 and 4 releases.

@bencroker
Copy link
Contributor Author

Thanks!

@brandonkelly
Copy link
Member

Craft 3.7.64 and 4.3.7 have been released with that change.

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

No branches or pull requests

2 participants