You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
TableDiff: Unicode characters are silently dropped from diff #136
The TableDiff first converts some of the string's characters into named entities using htmlentities(). The remaining characters are tried to be converted using iconv() (might behave differently depending on underlying system) from UTF-8 into ISO-8859-1.
In 0.1.16 this failed with Notice: iconv(): Detected an illegal character in input string if characters remained that have no equivalent in ISO-8859-1 and neither modifiers //TRANSLIT or //IGNORE have been used.
In 0.1.17 there was a change (see #134 and its corresponding PR) that added the iconv modifier //IGNORE. While the conversion now succeeds, it silently drops any characters that have no representation in ISO-8859-1.
In our application there is an editorial workflow that needs to rely on the diff output being complete, so silently dropping characters isn't an viable option.
Steps to reproduce
$old = '<table><tr><td>React with :-) and "joy".</td></tr></table>';
$new = '<table><tr><td>React with 🙂 and "joy".</td></tr></table>';
$htmlDiff = newHtmlDiff($old, $new);
echo$htmlDiff->build();
In 0.1.16 it fails (or might fail), in 0.1.17 the Unicode character 🙂 is silently dropped.
Curious how performance is affected by this change - the first priority for this library is accuracy of the diff, and the fact that we currently dropping characters silently is not acceptable, so even if this is slower it does not matter since it will be more accurate - but I wonder if it also improves performance here or makes it worse. Was going to look at how the test suite runs before/after this change
Activity
jschroed91 commentedon Sep 18, 2025
Curious how performance is affected by this change - the first priority for this library is accuracy of the diff, and the fact that we currently dropping characters silently is not acceptable, so even if this is slower it does not matter since it will be more accurate - but I wonder if it also improves performance here or makes it worse. Was going to look at how the test suite runs before/after this change
podarcis commentedon Sep 19, 2025
@jschroed91 According to a quick benchmark the alternative solution seems to be 45% faster.