Browse Source

Fix nodes order being sometimes mangled when rewriting emoji (#20677)

* Fix front-end emoji tests

* Fix nodes order being sometimes mangled when rewriting emoji
Claire 1 year ago
parent
commit
b22e1476ca

+ 21 - 21
app/javascript/mastodon/features/emoji/__tests__/emoji-test.js

@@ -11,8 +11,8 @@ describe('emoji', () => {
     });
 
     it('works with unclosed tags', () => {
-      expect(emojify('hello>')).toEqual('hello>');
-      expect(emojify('<hello')).toEqual('<hello');
+      expect(emojify('hello>')).toEqual('hello&gt;');
+      expect(emojify('<hello')).toEqual('');
     });
 
     it('works with unclosed shortcodes', () => {
@@ -22,23 +22,23 @@ describe('emoji', () => {
 
     it('does unicode', () => {
       expect(emojify('\uD83D\uDC69\u200D\uD83D\uDC69\u200D\uD83D\uDC66\u200D\uD83D\uDC66')).toEqual(
-        '<img draggable="false" class="emojione" alt="๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆโ€๐Ÿ‘ฆ" title=":woman-woman-boy-boy:" src="/emoji/1f469-200d-1f469-200d-1f466-200d-1f466.svg" />');
+        '<img draggable="false" class="emojione" alt="๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆโ€๐Ÿ‘ฆ" title=":woman-woman-boy-boy:" src="/emoji/1f469-200d-1f469-200d-1f466-200d-1f466.svg">');
       expect(emojify('๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง')).toEqual(
-        '<img draggable="false" class="emojione" alt="๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง" title=":man-woman-girl-girl:" src="/emoji/1f468-200d-1f469-200d-1f467-200d-1f467.svg" />');
-      expect(emojify('๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ" title=":woman-woman-boy:" src="/emoji/1f469-200d-1f469-200d-1f466.svg" />');
+        '<img draggable="false" class="emojione" alt="๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง" title=":man-woman-girl-girl:" src="/emoji/1f468-200d-1f469-200d-1f467-200d-1f467.svg">');
+      expect(emojify('๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ" title=":woman-woman-boy:" src="/emoji/1f469-200d-1f469-200d-1f466.svg">');
       expect(emojify('\u2757')).toEqual(
-        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" />');
+        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg">');
     });
 
     it('does multiple unicode', () => {
       expect(emojify('\u2757 #\uFE0F\u20E3')).toEqual(
-        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" /> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg" />');
+        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg"> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg">');
       expect(emojify('\u2757#\uFE0F\u20E3')).toEqual(
-        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" /><img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg" />');
+        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg"><img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg">');
       expect(emojify('\u2757 #\uFE0F\u20E3 \u2757')).toEqual(
-        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" /> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg" /> <img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" />');
+        '<img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg"> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg"> <img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg">');
       expect(emojify('foo \u2757 #\uFE0F\u20E3 bar')).toEqual(
-        'foo <img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg" /> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg" /> bar');
+        'foo <img draggable="false" class="emojione" alt="โ—" title=":exclamation:" src="/emoji/2757.svg"> <img draggable="false" class="emojione" alt="#๏ธโƒฃ" title=":hash:" src="/emoji/23-20e3.svg"> bar');
     });
 
     it('ignores unicode inside of tags', () => {
@@ -46,16 +46,16 @@ describe('emoji', () => {
     });
 
     it('does multiple emoji properly (issue 5188)', () => {
-      expect(emojify('๐Ÿ‘Œ๐ŸŒˆ๐Ÿ’•')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘Œ" title=":ok_hand:" src="/emoji/1f44c.svg" /><img draggable="false" class="emojione" alt="๐ŸŒˆ" title=":rainbow:" src="/emoji/1f308.svg" /><img draggable="false" class="emojione" alt="๐Ÿ’•" title=":two_hearts:" src="/emoji/1f495.svg" />');
-      expect(emojify('๐Ÿ‘Œ ๐ŸŒˆ ๐Ÿ’•')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘Œ" title=":ok_hand:" src="/emoji/1f44c.svg" /> <img draggable="false" class="emojione" alt="๐ŸŒˆ" title=":rainbow:" src="/emoji/1f308.svg" /> <img draggable="false" class="emojione" alt="๐Ÿ’•" title=":two_hearts:" src="/emoji/1f495.svg" />');
+      expect(emojify('๐Ÿ‘Œ๐ŸŒˆ๐Ÿ’•')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘Œ" title=":ok_hand:" src="/emoji/1f44c.svg"><img draggable="false" class="emojione" alt="๐ŸŒˆ" title=":rainbow:" src="/emoji/1f308.svg"><img draggable="false" class="emojione" alt="๐Ÿ’•" title=":two_hearts:" src="/emoji/1f495.svg">');
+      expect(emojify('๐Ÿ‘Œ ๐ŸŒˆ ๐Ÿ’•')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘Œ" title=":ok_hand:" src="/emoji/1f44c.svg"> <img draggable="false" class="emojione" alt="๐ŸŒˆ" title=":rainbow:" src="/emoji/1f308.svg"> <img draggable="false" class="emojione" alt="๐Ÿ’•" title=":two_hearts:" src="/emoji/1f495.svg">');
     });
 
     it('does an emoji that has no shortcode', () => {
-      expect(emojify('๐Ÿ‘โ€๐Ÿ—จ')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘โ€๐Ÿ—จ" title="" src="/emoji/1f441-200d-1f5e8.svg" />');
+      expect(emojify('๐Ÿ‘โ€๐Ÿ—จ')).toEqual('<img draggable="false" class="emojione" alt="๐Ÿ‘โ€๐Ÿ—จ" title="" src="/emoji/1f441-200d-1f5e8.svg">');
     });
 
     it('does an emoji whose filename is irregular', () => {
-      expect(emojify('โ†™๏ธ')).toEqual('<img draggable="false" class="emojione" alt="โ†™๏ธ" title=":arrow_lower_left:" src="/emoji/2199.svg" />');
+      expect(emojify('โ†™๏ธ')).toEqual('<img draggable="false" class="emojione" alt="โ†™๏ธ" title=":arrow_lower_left:" src="/emoji/2199.svg">');
     });
 
     it('avoid emojifying on invisible text', () => {
@@ -67,26 +67,26 @@ describe('emoji', () => {
 
     it('avoid emojifying on invisible text with nested tags', () => {
       expect(emojify('<span class="invisible">๐Ÿ˜„<span class="foo">bar</span>๐Ÿ˜ด</span>๐Ÿ˜‡'))
-        .toEqual('<span class="invisible">๐Ÿ˜„<span class="foo">bar</span>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg" />');
+        .toEqual('<span class="invisible">๐Ÿ˜„<span class="foo">bar</span>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg">');
       expect(emojify('<span class="invisible">๐Ÿ˜„<span class="invisible">๐Ÿ˜•</span>๐Ÿ˜ด</span>๐Ÿ˜‡'))
-        .toEqual('<span class="invisible">๐Ÿ˜„<span class="invisible">๐Ÿ˜•</span>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg" />');
-      expect(emojify('<span class="invisible">๐Ÿ˜„<br/>๐Ÿ˜ด</span>๐Ÿ˜‡'))
-        .toEqual('<span class="invisible">๐Ÿ˜„<br/>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg" />');
+        .toEqual('<span class="invisible">๐Ÿ˜„<span class="invisible">๐Ÿ˜•</span>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg">');
+      expect(emojify('<span class="invisible">๐Ÿ˜„<br>๐Ÿ˜ด</span>๐Ÿ˜‡'))
+        .toEqual('<span class="invisible">๐Ÿ˜„<br>๐Ÿ˜ด</span><img draggable="false" class="emojione" alt="๐Ÿ˜‡" title=":innocent:" src="/emoji/1f607.svg">');
     });
 
     it('skips the textual presentation VS15 character', () => {
       expect(emojify('โœด๏ธŽ')) // This is U+2734 EIGHT POINTED BLACK STAR then U+FE0E VARIATION SELECTOR-15
-        .toEqual('<img draggable="false" class="emojione" alt="โœด" title=":eight_pointed_black_star:" src="/emoji/2734_border.svg" />');
+        .toEqual('<img draggable="false" class="emojione" alt="โœด" title=":eight_pointed_black_star:" src="/emoji/2734_border.svg">');
     });
 
     it('does an simple emoji properly', () => {
       expect(emojify('โ™€โ™‚'))
-        .toEqual('<img draggable="false" class="emojione" alt="โ™€" title=":female_sign:" src="/emoji/2640.svg" /><img draggable="false" class="emojione" alt="โ™‚" title=":male_sign:" src="/emoji/2642.svg" />');
+        .toEqual('<img draggable="false" class="emojione" alt="โ™€" title=":female_sign:" src="/emoji/2640.svg"><img draggable="false" class="emojione" alt="โ™‚" title=":male_sign:" src="/emoji/2642.svg">');
     });
 
     it('does an emoji containing ZWJ properly', () => {
       expect(emojify('๐Ÿ’‚โ€โ™€๏ธ๐Ÿ’‚โ€โ™‚๏ธ'))
-        .toEqual('<img draggable="false" class="emojione" alt="๐Ÿ’‚\u200Dโ™€๏ธ" title=":female-guard:" src="/emoji/1f482-200d-2640-fe0f_border.svg" /><img draggable="false" class="emojione" alt="๐Ÿ’‚\u200Dโ™‚๏ธ" title=":male-guard:" src="/emoji/1f482-200d-2642-fe0f_border.svg" />');
+        .toEqual('<img draggable="false" class="emojione" alt="๐Ÿ’‚\u200Dโ™€๏ธ" title=":female-guard:" src="/emoji/1f482-200d-2640-fe0f_border.svg"><img draggable="false" class="emojione" alt="๐Ÿ’‚\u200Dโ™‚๏ธ" title=":male-guard:" src="/emoji/1f482-200d-2642-fe0f_border.svg">');
     });
   });
 });

+ 11 - 4
app/javascript/mastodon/features/emoji/emoji.js

@@ -19,10 +19,13 @@ const emojiFilename = (filename) => {
   return borderedEmoji.includes(filename) ? (filename + '_border') : filename;
 };
 
+const domParser = new DOMParser();
+
 const emojifyTextNode = (node, customEmojis) => {
-  const parentElement = node.parentElement;
   let str = node.textContent;
 
+  const fragment = new DocumentFragment();
+
   for (;;) {
     let match, i = 0;
 
@@ -64,12 +67,16 @@ const emojifyTextNode = (node, customEmojis) => {
       }
     }
 
+    fragment.append(document.createTextNode(str.slice(0, i)));
+    if (replacement) {
+      fragment.append(domParser.parseFromString(replacement, 'text/html').documentElement.getElementsByTagName('img')[0]);
+    }
     node.textContent = str.slice(0, i);
-    parentElement.insertAdjacentHTML('beforeend', replacement);
     str = str.slice(rend);
-    node = document.createTextNode(str);
-    parentElement.append(node);
   }
+
+  fragment.append(document.createTextNode(str));
+  node.parentElement.replaceChild(fragment, node);
 };
 
 const emojifyNode = (node, customEmojis) => {