mirror of
				https://github.com/zulip/zulip.git
				synced 2025-10-31 03:53:50 +00:00 
			
		
		
		
	thumbnail: Put the original dimensions on spinner images.
This lets us reserve the right amount of space in the message feed
immediately.
(cherry picked from commit 8bacdbc895)
			
			
This commit is contained in:
		
				
					committed by
					
						 Tim Abbott
						Tim Abbott
					
				
			
			
				
	
			
			
			
						parent
						
							c27f09f7c8
						
					
				
				
					commit
					94ff57a4d9
				
			| @@ -18,6 +18,16 @@ clients should check the `zulip_feature_level` field, present in the | ||||
| /register`](/api/register-queue) responses, to determine the API | ||||
| format used by the Zulip server that they are interacting with. | ||||
|  | ||||
| ## Changes in Zulip 9.2 | ||||
|  | ||||
| **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. | ||||
|  | ||||
| ## Changes in Zulip 9.0 | ||||
|  | ||||
| **Feature level 277** | ||||
|   | ||||
| @@ -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 | ||||
|   `{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 | ||||
| @@ -103,10 +108,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 | ||||
|   | ||||
| @@ -34,8 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" | ||||
| # new level means in api_docs/changelog.md, as well as "**Changes**" | ||||
| # entries in the endpoint's documentation in `zulip.yaml`. | ||||
|  | ||||
| API_FEATURE_LEVEL = 277  # Last bumped for Zulip 9.0 | ||||
|  | ||||
| API_FEATURE_LEVEL = 278  # Last bumped for backporting 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 | ||||
|   | ||||
| @@ -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) | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
		Reference in New Issue
	
	Block a user