From c230d2e44380915afa665c9ecd77e0f944a79288 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Mon, 28 Sep 2020 13:23:46 +0200
Subject: [PATCH] FIX: prevents scrollbar jumping in the emoji picker (#10749)

This was mostly due to `content-visibility: auto;` but setting the width/height is also a good practice.
---
 .../javascripts/pretty-text/addon/emoji.js    |  4 +++-
 app/assets/stylesheets/common/base/emoji.scss |  1 -
 test/javascripts/acceptance/topic-test.js     |  6 +++---
 test/javascripts/lib/emoji-test.js            | 20 +++++++++----------
 test/javascripts/lib/search-test.js           |  2 +-
 test/javascripts/models/topic-test.js         |  4 ++--
 6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/pretty-text/addon/emoji.js b/app/assets/javascripts/pretty-text/addon/emoji.js
index 4f588a2f4f5..67a7c2afaf1 100644
--- a/app/assets/javascripts/pretty-text/addon/emoji.js
+++ b/app/assets/javascripts/pretty-text/addon/emoji.js
@@ -126,7 +126,9 @@ export function performEmojiUnescape(string, opts) {
       isReplacableInlineEmoji(string, index, inlineEmoji);
 
     return url && isReplacable
-      ? `<img src='${url}' ${opts.skipTitle ? "" : `title='${emojiVal}'`} ${
+      ? `<img width="20" height="20" src='${url}' ${
+          opts.skipTitle ? "" : `title='${emojiVal}'`
+        } ${
           opts.lazy ? "loading='lazy' " : ""
         }alt='${emojiVal}' class='${classes}'>`
       : m;
diff --git a/app/assets/stylesheets/common/base/emoji.scss b/app/assets/stylesheets/common/base/emoji.scss
index 434e94cf3c4..ecbdaab49b8 100644
--- a/app/assets/stylesheets/common/base/emoji.scss
+++ b/app/assets/stylesheets/common/base/emoji.scss
@@ -65,7 +65,6 @@ sup img.emoji {
 
     .section {
       margin-bottom: 1em;
-      content-visibility: auto;
 
       .trash-recent {
         background: none;
diff --git a/test/javascripts/acceptance/topic-test.js b/test/javascripts/acceptance/topic-test.js
index d40bdb26deb..021389f7533 100644
--- a/test/javascripts/acceptance/topic-test.js
+++ b/test/javascripts/acceptance/topic-test.js
@@ -152,7 +152,7 @@ QUnit.test("Updating the topic title with emojis", async (assert) => {
 
   assert.equal(
     find(".fancy-title").html().trim(),
-    `emojis title <img src="/images/emoji/emoji_one/bike.png?v=${v}" title="bike" alt="bike" class="emoji"> <img src="/images/emoji/emoji_one/blonde_woman/6.png?v=${v}" title="blonde_woman:t6" alt="blonde_woman:t6" class="emoji">`,
+    `emojis title <img width=\"20\" height=\"20\" src="/images/emoji/emoji_one/bike.png?v=${v}" title="bike" alt="bike" class="emoji"> <img width=\"20\" height=\"20\" src="/images/emoji/emoji_one/blonde_woman/6.png?v=${v}" title="blonde_woman:t6" alt="blonde_woman:t6" class="emoji">`,
     "it displays the new title with emojis"
   );
 });
@@ -167,7 +167,7 @@ QUnit.test("Updating the topic title with unicode emojis", async (assert) => {
 
   assert.equal(
     find(".fancy-title").html().trim(),
-    `emojis title <img src="/images/emoji/emoji_one/man_farmer.png?v=${v}" title="man_farmer" alt="man_farmer" class="emoji"><img src="/images/emoji/emoji_one/pray.png?v=${v}" title="pray" alt="pray" class="emoji">`,
+    `emojis title <img width=\"20\" height=\"20\" src="/images/emoji/emoji_one/man_farmer.png?v=${v}" title="man_farmer" alt="man_farmer" class="emoji"><img width=\"20\" height=\"20\" src="/images/emoji/emoji_one/pray.png?v=${v}" title="pray" alt="pray" class="emoji">`,
     "it displays the new title with escaped unicode emojis"
   );
 });
@@ -185,7 +185,7 @@ QUnit.test(
 
     assert.equal(
       find(".fancy-title").html().trim(),
-      `Test<img src="/images/emoji/emoji_one/slightly_smiling_face.png?v=${v}" title="slightly_smiling_face" alt="slightly_smiling_face" class="emoji">Title`,
+      `Test<img width=\"20\" height=\"20\" src="/images/emoji/emoji_one/slightly_smiling_face.png?v=${v}" title="slightly_smiling_face" alt="slightly_smiling_face" class="emoji">Title`,
       "it displays the new title with escaped unicode emojis"
     );
   }
diff --git a/test/javascripts/lib/emoji-test.js b/test/javascripts/lib/emoji-test.js
index 3a3fed402bf..5fa7ea99861 100644
--- a/test/javascripts/lib/emoji-test.js
+++ b/test/javascripts/lib/emoji-test.js
@@ -32,12 +32,12 @@ QUnit.test("emojiUnescape", function (assert) {
   );
   testUnescape(
     "emoticons :)",
-    `emoticons <img src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
+    `emoticons <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
     "emoticons are still supported"
   );
   testUnescape(
     "With emoji :O: :frog: :smile:",
-    `With emoji <img src='/images/emoji/emoji_one/o.png?v=${v}' title='O' alt='O' class='emoji'> <img src='/images/emoji/emoji_one/frog.png?v=${v}' title='frog' alt='frog' class='emoji'> <img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
+    `With emoji <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/o.png?v=${v}' title='O' alt='O' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/frog.png?v=${v}' title='frog' alt='frog' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
     "title with emoji"
   );
   testUnescape(
@@ -47,27 +47,27 @@ QUnit.test("emojiUnescape", function (assert) {
   );
   testUnescape(
     "(:frog:) :)",
-    `(<img src='/images/emoji/emoji_one/frog.png?v=${v}' title='frog' alt='frog' class='emoji'>) <img src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
+    `(<img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/frog.png?v=${v}' title='frog' alt='frog' class='emoji'>) <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
     "non-word characters allowed next to emoji"
   );
   testUnescape(
     ":smile: hi",
-    `<img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> hi`,
+    `<img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> hi`,
     "start of line"
   );
   testUnescape(
     "hi :smile:",
-    `hi <img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
+    `hi <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
     "end of line"
   );
   testUnescape(
     "hi :blonde_woman:t4:",
-    `hi <img src='/images/emoji/emoji_one/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'>`,
+    `hi <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'>`,
     "support for skin tones"
   );
   testUnescape(
     "hi :blonde_woman:t4: :blonde_man:t6:",
-    `hi <img src='/images/emoji/emoji_one/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'> <img src='/images/emoji/emoji_one/blonde_man/6.png?v=${v}' title='blonde_man:t6' alt='blonde_man:t6' class='emoji'>`,
+    `hi <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/blonde_man/6.png?v=${v}' title='blonde_man:t6' alt='blonde_man:t6' class='emoji'>`,
     "support for multiple skin tones"
   );
   testUnescape(
@@ -95,7 +95,7 @@ QUnit.test("emojiUnescape", function (assert) {
   );
   testUnescape(
     "Hello 😊 World",
-    `Hello <img src='/images/emoji/emoji_one/blush.png?v=${v}' title='blush' alt='blush' class='emoji'> World`,
+    `Hello <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/blush.png?v=${v}' title='blush' alt='blush' class='emoji'> World`,
     "emoji from Unicode emoji"
   );
   testUnescape(
@@ -108,7 +108,7 @@ QUnit.test("emojiUnescape", function (assert) {
   );
   testUnescape(
     "Hello😊World",
-    `Hello<img src='/images/emoji/emoji_one/blush.png?v=${v}' title='blush' alt='blush' class='emoji'>World`,
+    `Hello<img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/blush.png?v=${v}' title='blush' alt='blush' class='emoji'>World`,
     "emoji from Unicode emoji when inline translation enabled",
     {
       enable_inline_emoji_translation: true,
@@ -124,7 +124,7 @@ QUnit.test("emojiUnescape", function (assert) {
   );
   testUnescape(
     "hi:smile:",
-    `hi<img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
+    `hi<img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
     "emoji when inline translation enabled",
     { enable_inline_emoji_translation: true }
   );
diff --git a/test/javascripts/lib/search-test.js b/test/javascripts/lib/search-test.js
index fadbad9e185..075ebcec944 100644
--- a/test/javascripts/lib/search-test.js
+++ b/test/javascripts/lib/search-test.js
@@ -32,7 +32,7 @@ QUnit.test("unescapesEmojisInBlurbs", (assert) => {
   const blurb = results.posts[0].get("blurb");
 
   assert.ok(blurb.indexOf("thinking.png"));
-  assert.ok(blurb.indexOf("<img src") === 0);
+  assert.ok(blurb.indexOf('<img width="20" height="20" src') === 0);
   assert.ok(blurb.indexOf(":thinking:") === -1);
 });
 
diff --git a/test/javascripts/models/topic-test.js b/test/javascripts/models/topic-test.js
index db5adf2c0bf..47ecd51f5b1 100644
--- a/test/javascripts/models/topic-test.js
+++ b/test/javascripts/models/topic-test.js
@@ -129,7 +129,7 @@ QUnit.test("fancyTitle", (assert) => {
 
   assert.equal(
     topic.get("fancyTitle"),
-    `<img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> with all <img src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'> the emojis <img src='/images/emoji/emoji_one/pear.png?v=${v}' title='pear' alt='pear' class='emoji'><img src='/images/emoji/emoji_one/peach.png?v=${v}' title='peach' alt='peach' class='emoji'>`,
+    `<img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> with all <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'> the emojis <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/pear.png?v=${v}' title='pear' alt='pear' class='emoji'><img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/peach.png?v=${v}' title='peach' alt='peach' class='emoji'>`,
     "supports emojis"
   );
 });
@@ -159,7 +159,7 @@ QUnit.test("excerpt", (assert) => {
 
   assert.equal(
     topic.get("escapedExcerpt"),
-    `This is a test topic <img src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
+    `This is a test topic <img width=\"20\" height=\"20\" src='/images/emoji/emoji_one/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
     "supports emojis"
   );
 });