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

Use PHP built-in stripslashes for PS 8.0.0+ #74

Merged

Conversation

leemyongpakvn
Copy link
Contributor

Questions Answers
Description? Tools::stripslashes() is deprecated since version 8.0.0. Use PHP's stripslashes instead.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#34770.
How to test? Apply this PR change, then follow the related issue's Steps to reproduce to see the error gone.

@@ -567,12 +567,13 @@ public function sendMessage()
&& empty($mailAlreadySend)
&& ($sendConfirmationEmail || $sendNotificationEmail)
) {
$message = version_compare(_PS_VERSION_, '8.0.0', '>=') ? stripslashes($message) : Tools::stripslashes($message);
Copy link
Contributor

Choose a reason for hiding this comment

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

As this PR targets develop branch, I think that you can use built-in stripslashes() and remove the check on PrestaShop version.

Copy link
Contributor Author

@leemyongpakvn leemyongpakvn Dec 18, 2023

Choose a reason for hiding this comment

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

I think dev branch in this repo is somehow different from develop branch in Core repo :)
develop branch in Core repo means PS 9 for now, but dev branch in this repo supports PS 1.7.2+

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my bad, I thought I was on prestashop repo.

Copy link
Contributor

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

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

Hello @leemyongpakvn ,

Tested with 8.1.x and 9.0.x.

Tested with logged in customer and guest. Tested with attachment or not. Tested with order reference or not.

LGTM ✅

@nicosomb nicosomb merged commit 2554f56 into PrestaShop:dev Dec 19, 2023
12 checks passed
@florine2623 florine2623 mentioned this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
4 participants