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

ext-mbstring should be required #203

Open
alecpl opened this issue Mar 28, 2021 · 2 comments
Open

ext-mbstring should be required #203

alecpl opened this issue Mar 28, 2021 · 2 comments

Comments

@alecpl
Copy link
Contributor

alecpl commented Mar 28, 2021

Here's why:

  1. Masterminds\HTML5\Parser\CharacterReference::lookupDecimal() uses mb_decode_numericentity() unconditionally.
  2. Looking at Masterminds\HTML5\Parser\UTF8Utils::convertToUTF8() either iconv or mbstring must be available (if the input encoding is not 'auto').

This would allow to:

  1. Get rid of iconv() use. In my experience mbstring is really a better solution.
  2. Remove use of utf8_decode() which is not really valid and not needed when mbstring is available.
  3. Get rid of the fallback code.
@alecpl
Copy link
Contributor Author

alecpl commented Aug 6, 2022

Actually utf8_decode() is deprecated in PHP 8.2, and will be removed later. So, this is more like a bug now.

@goetas
Copy link
Member

goetas commented Jan 11, 2023

sorry for the late reply. makes sense what you are suggesting. would be happy to see a PR

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

No branches or pull requests

2 participants