thumbnail: Put the original dimensions on spinner images.

This lets us reserve the right amount of space in the message feed
immediately.

api_docs/changelog.md

@@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 10.0
 
+**Feature level 287**
+
+* [Markdown message
+  formatting](/api/message-formatting#image-previews): Added
+  `data-original-dimensions` attributes to placeholder images
+  (`image-loading-placeholder`), containing the dimensions of the
+  original image. This change was also backported to the Zulip 9.x
+  series, at feature level 278.
+
 **Feature level 286**
 
 * [`POST /user_uploads`](/api/upload-file): Added `filename` field to
@@ -87,6 +96,14 @@ releases.
 
 ## Changes in Zulip 9.0
 
+**Feature level 278**
+
+* [Markdown message
+  formatting](/api/message-formatting#image-previews): Added
+  `data-original-dimensions` attributes to placeholder images
+  (`image-loading-placeholder`), containing the dimensions of the
+  original image. Backported change from feature level 287.
+
 **Feature level 277**
 
 No changes; feature level used for Zulip 9.0 release.

api_docs/message-formatting.md

@@ -47,7 +47,7 @@ desired. For example:
 ``` html
 <div class="message_inline_image">
     <a href="/user_uploads/path/to/image.png" title="image.png">
-        <img class="image-loading-placeholder" src="/path/to/spinner.png">
+        <img class="image-loading-placeholder" data-original-dimensions="1920x1080" src="/path/to/spinner.png">
     </a>
 </div>
 ```
@@ -69,6 +69,13 @@ and triggers a message update.
 Clients are recommended to do the following when processing image
 previews:
 
+- Clients that would like to use the image's aspect ratio to lay out
+  one or more images in the message feed may use the
+  `data-original-dimensions` attribute, which is present even if the
+  image is a placeholder spinner.  This attribute encodes the
+  dimensions of the original image as `{width}x{height}`.  These
+  dimensions are for the image as rendered, _after_ any EXIF rotation
+  and mirroring has been applied.
 - If the client would like to control the thumbnail resolution used,
   it can replace the final section of the URL (`840x560.webp` in the
   example above) with the `name` of its preferred format from the set
@@ -85,9 +92,7 @@ previews:
   thumbnail from the server, to be transparently swapped in once it is
   available. Clients that would like to size the lightbox based on the
   size of the original image can use the `data-original-dimensions`
-  attribute, which encodes the dimensions of the original image as
+  attribute, as described above.
-  `{width}x{height}`, to do so.  These dimensions are for the image as
-  rendered, _after_ any EXIF rotation and mirroring has been applied.
 - Animated images will have a `data-animated` attribute on the `img`
   tag. As detailed in `server_thumbnail_formats`, both animated and
   still images are available for clients to use, depending on their
@@ -102,10 +107,14 @@ previews:
   format match what they requested.
 - No other processing of the URLs is recommended.
 
-**Changes**: In Zulip 9.0 (feature level 276), added
+**Changes**: In Zulip 9.2 (feature levels 278-279, and 287+), added
-`data-original-dimensions` attribute to images that have been
+`data-original-dimensions` to the `image-loading-placeholder` spinner
-thumbnailed, containing the dimensions of the full-size version of the
+images, containing the dimensions of the original image.
-image. Thumbnailing itself was reintroduced at feature level 275.
+
+In Zulip 9.0 (feature level 276), added `data-original-dimensions`
+attribute to images that have been thumbnailed, containing the
+dimensions of the full-size version of the image. Thumbnailing itself
+was reintroduced at feature level 275.
 
 Previously, with the exception of Zulip servers that used the beta
 Thumbor-based implementation years ago, all image previews in Zulip

version.py

@@ -35,7 +35,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
 # entries in the endpoint's documentation in `zulip.yaml`.
 
 
-API_FEATURE_LEVEL = 286  # Last bumped for uploaded filename
+API_FEATURE_LEVEL = 287  # original-dimensions on spinner
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/markdown/__init__.py

@@ -130,7 +130,7 @@ class DbData:
     sent_by_bot: bool
     stream_names: dict[str, int]
     translate_emoticons: bool
-    user_upload_previews: dict[str, MarkdownImageMetadata | None]
+    user_upload_previews: dict[str, MarkdownImageMetadata]
 
 
 # Format version of the Markdown rendering; stored along with rendered
@@ -628,6 +628,7 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
             # (even if that's "no preview yet") from the database
             # before rendering; is_image should have enforced that.
             assert path_id in self.zmd.zulip_db_data.user_upload_previews
+            metadata = self.zmd.zulip_db_data.user_upload_previews[path_id]
 
             # Insert a placeholder image spinner.  We post-process
             # this content (see rewrite_thumbnailed_images in
@@ -636,6 +637,10 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
             # already exists when the message is sent.
             img.set("class", "image-loading-placeholder")
             img.set("src", "/static/images/loading/loader-black.svg")
+            img.set(
+                "data-original-dimensions",
+                f"{metadata.original_width_px}x{metadata.original_height_px}",
+            )
         else:
             img.set("src", image_url)
 

zerver/lib/thumbnail.py

@@ -327,7 +327,7 @@ def split_thumbnail_path(file_path: str) -> tuple[str, BaseThumbnailFormat]:
 
 @dataclass
 class MarkdownImageMetadata:
-    url: str
+    url: str | None
     is_animated: bool
     original_width_px: int
     original_height_px: int
@@ -339,13 +339,13 @@ def get_user_upload_previews(
     lock: bool = False,
     enqueue: bool = True,
     path_ids: list[str] | None = None,
-) -> dict[str, MarkdownImageMetadata | None]:
+) -> dict[str, MarkdownImageMetadata]:
     if path_ids is None:
         path_ids = re.findall(r"/user_uploads/(\d+/[/\w.-]+)", content)
     if not path_ids:
         return {}
 
-    upload_preview_data: dict[str, MarkdownImageMetadata | None] = {}
+    upload_preview_data: dict[str, MarkdownImageMetadata] = {}
 
     image_attachments = ImageAttachment.objects.filter(realm_id=realm_id, path_id__in=path_ids)
     if lock:
@@ -355,7 +355,12 @@ def get_user_upload_previews(
             # Image exists, and header of it parsed as a valid image,
             # but has not been thumbnailed yet; we will render a
             # spinner.
-            upload_preview_data[image_attachment.path_id] = None
+            upload_preview_data[image_attachment.path_id] = MarkdownImageMetadata(
+                url=None,
+                is_animated=False,
+                original_width_px=image_attachment.original_width_px,
+                original_height_px=image_attachment.original_height_px,
+            )
 
             # We re-queue the row for thumbnailing to make sure that
             # we do eventually thumbnail it (e.g. if this is a
@@ -410,7 +415,7 @@ html_formatter = HTMLFormatter(
 
 def rewrite_thumbnailed_images(
     rendered_content: str,
-    images: dict[str, MarkdownImageMetadata | None],
+    images: dict[str, MarkdownImageMetadata],
     to_delete: set[str] | None = None,
 ) -> tuple[str | None, set[str]]:
     if not images and not to_delete:
@@ -449,10 +454,13 @@ def rewrite_thumbnailed_images(
 
         image_data = images.get(path_id)
         if image_data is None:
-            # Has not been thumbnailed yet; leave it as a spinner.
+            # The message has multiple images, and we're updating just
-            # This happens routinely when a message contained multiple
+            # one image, and it's not this one.  Leave this one as-is.
-            # unthumbnailed images, and only one of those images just
+            remaining_thumbnails.add(path_id)
-            # completed thumbnailing.
+        elif image_data.url is None:
+            # We're re-rendering the whole message, so fetched all of
+            # the image metadata rows; this is one of the images we
+            # about, but is not thumbnailed yet.
             remaining_thumbnails.add(path_id)
         else:
             changed = True

zerver/tests/test_markdown_thumbnail.py

@@ -123,7 +123,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
             expected = (
                 f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
                 f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
-                '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+                '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
             )
 
             message_id = self.send_message_content(content)
@@ -203,9 +203,9 @@ class MarkdownThumbnailTest(ZulipTestCase):
                 f'<p><a href="/user_uploads/{first_path_id}">first image</a><br>\n'
                 f'<a href="/user_uploads/{second_path_id}">second image</a></p>\n'
                 f'<div class="message_inline_image"><a href="/user_uploads/{first_path_id}" title="first image">'
-                '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+                '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
                 f'<div class="message_inline_image"><a href="/user_uploads/{second_path_id}" title="second image">'
-                '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+                '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
             ),
         )
 
@@ -217,7 +217,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
                 f'<p><a href="/user_uploads/{first_path_id}">first image</a><br>\n'
                 f'<a href="/user_uploads/{second_path_id}">second image</a></p>\n'
                 f'<div class="message_inline_image"><a href="/user_uploads/{first_path_id}" title="first image">'
-                '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+                '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
                 f'<div class="message_inline_image"><a href="/user_uploads/{second_path_id}" title="second image">'
                 f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{second_path_id}/840x560.webp"></a></div>'
             ),
@@ -304,7 +304,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
         )
         placeholder = (
             f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
-            '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+            '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
         )
         self.assert_message_content_is(
             channel_message_id,
@@ -363,7 +363,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
         expected = (
             f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
             f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
-            '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+            '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
         )
         self.assertEqual(send_request.message.rendered_content, expected)
 
@@ -398,7 +398,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
             expected = (
                 f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
                 f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
-                '<img class="image-loading-placeholder" src="/static/images/loading/loader-black.svg"></a></div>'
+                '<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
             )
 
             message_id = self.send_message_content(content)