Fix Unicode handling in table diff by encoding non-ASCII chars as numeric entities #137
Conversation
…eric entities
Previously, `TableDiff::createDocumentWithHtml()` attempted to normalize
text for `DOMDocument::loadHTML()` by passing it through `htmlentities()`
and `iconv('UTF-8', 'ISO-8859-1//IGNORE', ...)`. This caused non-ASCII
characters (e.g. emoji, Cyrillic, CJK) that are not representable in
ISO-8859-1 to be dropped entirely.
This patch replaces that logic with a call to `mb_encode_numericentity()`
to convert all non-ASCII Unicode codepoints (U+0080 – U+10FFFF) into
decimal HTML numeric entities. This ensures that the full Unicode range
is preserved and parsed correctly by `DOMDocument`, which otherwise
defaults to ISO-8859-1.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes Unicode handling in table diff functionality by replacing the problematic character encoding approach with numeric HTML entities. The change ensures that non-ASCII characters (emoji, Cyrillic, CJK, etc.) are preserved instead of being dropped during HTML processing.
- Replaces
htmlentities()andiconv()chain withmb_encode_numericentity()for better Unicode support - Converts all non-ASCII Unicode codepoints (U+0080 – U+10FFFF) to decimal HTML numeric entities
- Simplifies the HTML loading process by removing the complex encoding/decoding chain
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $convmap = [ | ||
| 0x80, // Leave ASCII range intact | ||
| 0x10FFFF, // Convert the rest of the Unicode range | ||
| 0, | ||
| 0xFFFFFF, | ||
| ]; |
There was a problem hiding this comment.
The convmap array uses magic numbers without clear documentation. Consider defining named constants for these Unicode range boundaries (0x80 for ASCII_END, 0x10FFFF for UNICODE_MAX) to improve code readability and maintainability.
Copilot uses AI. Check for mistakes.
|
Looks like our tests are failing not because of the changes here but because we're using outdated composer version which no longer works as of Sept 1st 2025. Could be other reasons as well, I only glanced at the error logs in scrutinizer. Will need to come back to it Will make an issue to migrate to github actions for CI |
Previously,
TableDiff::createDocumentWithHtml()attempted to normalize text forDOMDocument::loadHTML()by passing it throughhtmlentities()andiconv('UTF-8', 'ISO-8859-1//IGNORE', ...). This caused non-ASCII characters (e.g. emoji, Cyrillic, CJK) that are neither representable HTML named entities nor in ISO-8859-1 to be dropped entirely.This patch replaces that logic with a call to
mb_encode_numericentity()to convert all non-ASCII Unicode codepoints (U+0080 – U+10FFFF) into decimal HTML numeric entities. This ensures that the full Unicode range is preserved and parsed correctly byDOMDocument, which otherwise defaults to ISO-8859-1.Resolves #136