-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Add multibyte support for attributeSource getOptionId method #22033
Add multibyte support for attributeSource getOptionId method #22033
Conversation
* @param null|string $encoding | ||
* @return int|\\lt | ||
*/ | ||
private function mbStrcasecmp($str1, $str2, $encoding = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
why do you add an optional parameter if it's never used? Or am I missing something?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @gomenca, do you have any update on this? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aleron75 , you are right. I've just used this function in another places and used this parameter. I drop it.
Hi @aleron75, thank you for the review. |
✔️ QA passed |
Hi @gomencal, thank you for your contribution! |
Add multibyte support for attributeSource getOptionId method
Description (*)
Currently, if a option value has special characters in UTF-8 (for instance), the use of strcasecmp PHP function causes incorrect matching, because it doesn't support multibyte strings.
Fixed Issues (if relevant)
Manual testing scenarios (*)
$value = 'Sí'; $attribute = $this->resource->getAttribute($attributeCode); $optionId = $attribute->getSource()->getOptionId($value);
It must find the option value and return its Id.
Contribution checklist (*)