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

Update Options.php #8130

Merged
merged 2 commits into from
Mar 14, 2017
Merged

Update Options.php #8130

merged 2 commits into from
Mar 14, 2017

Conversation

redelschaap
Copy link
Contributor

@redelschaap redelschaap commented Jan 13, 2017

Prevent special charachters (like ü or ß) from being inserted as HTML encoded entities (like ü or ß) in widget input fields.

Prevent special charachters (like ü or ß) from being inserted as HTML encoded entities (like ü or ß) in widget input fields.
Copy link
Contributor

@vrann vrann left a comment

Choose a reason for hiding this comment

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

Can you accompany pull request with the unit tests, please?

@@ -163,6 +163,8 @@ protected function _addField($parameter)
}
}

$data['value'] = html_entity_decode( $data['value'] );
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Please use \Magento\Framework\Escaper::escapeHtmlAttr instead of native PHP library
  • there is extra space after "(", please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The \Magento\Framework\Escaper::escapeHtmlAttr function doesn't give the desired result. html_entity_decode is used by Magento core elsewhere. See https://github.com/magento/magento2/search?utf8=✓&q=html_entity_decode&type=Code. I have removed the extra spaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, looks good

@vrann
Copy link
Contributor

vrann commented Mar 1, 2017

@redelschaap Thank you for the contribution! Left couple comments in the code review.

Removed spaces
@magento-team magento-team merged commit 3b3e030 into magento:develop Mar 14, 2017
@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@redelschaap merged! Appreciate your support.

@redelschaap
Copy link
Contributor Author

Great! Thanks!

@skovalenk
Copy link
Contributor

@vrann @redelschaap
Why you assume that the data['value'] always will be a string?
What will happens in case of multiselect?

@redelschaap
Copy link
Contributor Author

Than that value is serialized, so it will be a string on that point. Right @vrann?

magento-devops-reposync-svc pushed a commit that referenced this pull request Feb 13, 2023
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 this pull request may close these issues.

4 participants