diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 12808b7a1a..55fc96c0c2 100644 --- a/api_docs/changelog.md +++ b/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. diff --git a/api_docs/message-formatting.md b/api_docs/message-formatting.md index 238173c8cc..2f0cbad79e 100644 --- a/api_docs/message-formatting.md +++ b/api_docs/message-formatting.md @@ -47,7 +47,7 @@ desired. For example: ``` html
- +
``` @@ -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 - `{width}x{height}`, to do so. These dimensions are for the image as - rendered, _after_ any EXIF rotation and mirroring has been applied. + attribute, as described above. - 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 -`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. +**Changes**: In Zulip 9.2 (feature levels 278-279, and 287+), added +`data-original-dimensions` to the `image-loading-placeholder` spinner +images, containing the dimensions of the original image. + +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 diff --git a/version.py b/version.py index 4670dc7f6d..b334455abe 100644 --- a/version.py +++ b/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 diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py index 6ecf9963a3..a9c56cf297 100644 --- a/zerver/lib/markdown/__init__.py +++ b/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) diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py index 3e55c11951..f15303fdf4 100644 --- a/zerver/lib/thumbnail.py +++ b/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. - # This happens routinely when a message contained multiple - # unthumbnailed images, and only one of those images just - # completed thumbnailing. + # The message has multiple images, and we're updating just + # one image, and it's not this one. Leave this one as-is. + remaining_thumbnails.add(path_id) + 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 diff --git a/zerver/tests/test_markdown_thumbnail.py b/zerver/tests/test_markdown_thumbnail.py index 1ebea1654b..07ac40d598 100644 --- a/zerver/tests/test_markdown_thumbnail.py +++ b/zerver/tests/test_markdown_thumbnail.py @@ -123,7 +123,7 @@ class MarkdownThumbnailTest(ZulipTestCase): expected = ( f'

image

\n' f'
' - '
' + '' ) message_id = self.send_message_content(content) @@ -203,9 +203,9 @@ class MarkdownThumbnailTest(ZulipTestCase): f'

first image
\n' f'second image

\n' f'
' - '
' + '' f'
' - '
' + '' ), ) @@ -217,7 +217,7 @@ class MarkdownThumbnailTest(ZulipTestCase): f'

first image
\n' f'second image

\n' f'
' - '
' + '' f'
' f'
' ), @@ -304,7 +304,7 @@ class MarkdownThumbnailTest(ZulipTestCase): ) placeholder = ( f'
' - '
' + '' ) self.assert_message_content_is( channel_message_id, @@ -363,7 +363,7 @@ class MarkdownThumbnailTest(ZulipTestCase): expected = ( f'

image

\n' f'
' - '
' + '' ) self.assertEqual(send_request.message.rendered_content, expected) @@ -398,7 +398,7 @@ class MarkdownThumbnailTest(ZulipTestCase): expected = ( f'

image

\n' f'
' - '
' + '' ) message_id = self.send_message_content(content)