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

Product custom options always selected to "Yes" in PHP8.1 #2440

Closed
alexherbs opened this issue Aug 17, 2022 · 6 comments · Fixed by #2441
Closed

Product custom options always selected to "Yes" in PHP8.1 #2440

alexherbs opened this issue Aug 17, 2022 · 6 comments · Fixed by #2441

Comments

@alexherbs
Copy link

alexherbs commented Aug 17, 2022

The custom options are always selected to yes, even if "No" is selected. The reason is the since PHP 8.1 the JSON data.is_requiere is 0 instead of "0" and the code below isn't processed any more, because the condition is false.

The line just needs to be changed to

if (data.is_require != undefined) {

@alexherbs alexherbs changed the title Custom Options always selected to "Yes" Product custom options always selected to "Yes" Aug 17, 2022
@alexherbs alexherbs changed the title Product custom options always selected to "Yes" Product custom options always selected to "Yes" in PHP8.1 Aug 17, 2022
@fballiano
Copy link
Contributor

Hi and thanks for the report, I've created #2441 to fix the problem, it's a slightly different implementation (to be more consistent with the past codebase) but it should work.

@kiatng
Copy link
Contributor

kiatng commented Aug 18, 2022

I am testing with PHP 8.1.8, but I cannot replicate the issue:
image
The Is Required dropdown is correctly set after saved. See screenshot above. What did I miss?

@kiatng
Copy link
Contributor

kiatng commented Aug 18, 2022

I take a look at the js:

//set selected is_require
if (data.is_require) {
$A($('<?php echo $this->getFieldId() ?>_'+data.id+'_is_require').options).each(function(option){
if (option.value==data.is_require) option.selected = true;
});
}

My analysis:
line 76: It is checking if the property is_require exists; string "0" or "1" is truthy, but int 0 is falsy; in my case, it is a string, that's why it works for me. However, it is no guarantee.
line 77: It iterates the options in the Is Required dropdown
line 78: Here it sets the selected attribute, it is correctly set in my test shown in the screenshot above

OT, to step through the js in Firefox, I needed to add //# sourceURL=option-panel before </script>, see stackoverflow.

But it is a good idea to have a proper check on line 76.

@alexherbs
Copy link
Author

@kiatng @fballiano Hi, thanks for your fast testing and your reply. I've two enviroments, one with PHP7 and one with PHP8.1 and in PHP7 I get "0" and in PHP8.1 i get INT 0 for "is_requiere" which caused lots of problems with my products. Maybe some PHP settings causes different behaviour in JSON enconding ?!? But, its truly fair to check by an proper way. Thanks at all.

@fballiano
Copy link
Contributor

mmm i'm not sure why but on my php8.1 environment I could reproduce the problem immediately.

@Sdfendor
Copy link
Contributor

Sdfendor commented Aug 18, 2022

The "problem" (or behavior change) lies deeper than JSON encoding. In PHP 8.1 all integers from database/PDO so e.g. IDs are no longer strings but integers. This is then handed down and results in this case in "is_required" as an int. I didn't do a deep dive yet, at which point in the code this change happens specifically though (is it already changed in built-in PHP or does the change occur in OpenMage implementation regarding database).

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

Successfully merging a pull request may close this issue.

5 participants