-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Fix Incorrect Emoji Rendering #15830
base: master
Are you sure you want to change the base?
Conversation
Hi, thanks for your contribution! |
@@ -199,6 +200,43 @@ class ChatInput extends Component<IProps, IState> { | |||
|
|||
} | |||
|
|||
|
|||
_sanitizeMessage(text: string): string { |
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.
Can you explain what approach you took here?
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.
Can you explain what approach you took here?
I created the _sanitizeMessage function to properly handle IPv6 addresses, timestamps, and prevent unintended emoji conversion. However, the IPv6 address solution was affecting other emojis, preventing them from rendering correctly. To fix this, I introduced an emoji map to ensure every emoji is processed accurately. I placed _sanitizeMessage before trimming to ensure proper handling before sending the message.
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.
Can you explain what approach you took here?
I created the _sanitizeMessage function to properly handle IPv6 addresses, timestamps, and prevent unintended emoji conversion. However, the IPv6 address solution was affecting other emojis, preventing them from rendering correctly. To fix this, I introduced an emoji map to ensure every emoji is processed accurately. I placed _sanitizeMessage before trimming to ensure proper handling before sending the message.
The _sanitizeMessage function processes the input message to ensure correct formatting before sending. It first wraps IPv6 addresses and timestamps in backticks to prevent unintended emoji conversion. Then, it converts valid emoji codes like ❤️ and classic smileys like :P or :D into their respective emojis. Finally, it prevents accidental emoji conversions in regular text by ensuring : is not mistakenly treated as an emoji trigger.
The function returns a cleaned-up message string where all intended emojis render correctly while preserving special cases like IP addresses and timestamps.
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.
I don't think that is a good solution, there could be arbitrary text that it's not IP addresses. Not to mention the performance impact of applying so many regexes to every message.
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.
I don't think that is a good solution, there could be arbitrary text that it's not IP addresses. Not to mention the performance impact of applying so many regexes to every message.
Sure, let me see if I can think of a better way or improve this one.
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 @saghul, instead of using regex to handle the emoji rendering issue, we can take a different approach by modifying the output of the toArray function directly within the Message component. This allows us to control how text and emojis are processed at the rendering stage, ensuring that only valid emoji patterns are transformed while preserving other text elements like IPv6 addresses. This approach improves efficiency and maintains message integrity without relying on multiple regex checks.
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.
what's your take on this ?
FIxes Issue Number #15824
Fix Description:
Prevented unintended emoji conversion for 💯 and other emojis in IPv6 addresses by introducing backticks.
Ensured proper rendering of classic smileys like :P, :D, and :).
Refined regex handling to accurately distinguish between emoji codes and regular text.
Fixed edge cases where certain symbols incorrectly triggered emoji rendering.