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

Revert "[stable24] Include more emoji chars to test and fixes after include it" #32426

Closed
wants to merge 1 commit into from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented May 16, 2022

Reverts #32256

I missed (scrolled over) related failing tests

@vitormattos
Copy link
Contributor

The failed tests was because in master branch (25), the apps/user_status/lib/Service/EmojiService.php was replaced by lib/private/Comments/EmojiHelper.php (#31703) and in PR #32256 I changed only the file lib/private/Comments/EmojiHelper.php but in 24 release the app user_status don't use the EmojiHelper for all and I think don't will use.

The fix for #32256 is duplicate the content of method isValidEmoji, that I wrong put into EmojiHelper, to EmojiService or call EmojiHelper->isValidEmoji in EmojiService->isValidEmoji.

@vitormattos
Copy link
Contributor

vitormattos commented May 16, 2022

Sorry for my mistake.

Only this solve the tests and remove the duplicated code:

diff --git a/apps/user_status/lib/Service/EmojiService.php b/apps/user_status/lib/Service/EmojiService.php
index 0f19793387..9254c250b6 100644
--- a/apps/user_status/lib/Service/EmojiService.php
+++ b/apps/user_status/lib/Service/EmojiService.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
  */
 namespace OCA\UserStatus\Service;
 
+use OC\Comments\EmojiHelper;
 use OCP\IDBConnection;
 
 /**
@@ -60,43 +61,7 @@ class EmojiService {
         * @return bool
         */
        public function isValidEmoji(string $emoji): bool {
-               $intlBreakIterator = \IntlBreakIterator::createCharacterInstance();
-               $intlBreakIterator->setText($emoji);
-
-               $characterCount = 0;
-               while ($intlBreakIterator->next() !== \IntlBreakIterator::DONE) {
-                       $characterCount++;
-               }
-
-               if ($characterCount !== 1) {
-                       return false;
-               }
-
-               $codePointIterator = \IntlBreakIterator::createCodePointInstance();
-               $codePointIterator->setText($emoji);
-
-               foreach ($codePointIterator->getPartsIterator() as $codePoint) {
-                       $codePointType = \IntlChar::charType($codePoint);
-
-                       // If the current code-point is an emoji or a modifier (like a skin-tone)
-                       // just continue and check the next character
-                       if ($codePointType === \IntlChar::CHAR_CATEGORY_MODIFIER_SYMBOL ||
-                               $codePointType === \IntlChar::CHAR_CATEGORY_MODIFIER_LETTER ||
-                               $codePointType === \IntlChar::CHAR_CATEGORY_OTHER_SYMBOL ||
-                               $codePointType === \IntlChar::CHAR_CATEGORY_GENERAL_OTHER_TYPES) {
-                               continue;
-                       }
-
-                       // If it's neither a modifier nor an emoji, we only allow
-                       // a zero-width-joiner or a variation selector 16
-                       $codePointValue = \IntlChar::ord($codePoint);
-                       if ($codePointValue === 8205 || $codePointValue === 65039) {
-                               continue;
-                       }
-
-                       return false;
-               }
-
-               return true;
+               $emojiHelper = new EmojiHelper($this->db);
+               return $emojiHelper->isValidEmoji($emoji);
        }
 }

@blizzz
Copy link
Member Author

blizzz commented May 16, 2022

@vitormattos then I'll merge the revert nevertheless and you do this port for 24 as a seperate PR?

@vitormattos
Copy link
Contributor

This is a possible solution.

@blizzz blizzz closed this May 16, 2022
@nickvergessen nickvergessen deleted the revert-32256-backport/32220/stable24 branch May 16, 2022 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants