markdown: Add support for  syntax for images.

[N-Shar-ma]

markdown: Render inline images as small lightbox previews in place.

Images added using the ![alt text](url) syntax are rendered in a fixed size, expandable via lightbox, exactly like the linked image previews, except inline.

For drafts which are only rendered by the frontend markdown processor, images are rendered as a regular link, for now.

markdown: Add support for ![alt text](url) syntax for inline images.

The standard markdown syntax for images is now supported in Zulip. Unlike linked image previews (which is the only way images could be added so far), images added using the ![alt text](url) syntax are truly inline, so are placed in the message exactly where the syntax is, not in a separate paragraph.

Fixes: #28912.

upload: Rename variable filename_url to syntax_to_insert.

This variable holds the markdown syntax for a file that's to be inserted into a message, hence the new name.

copy_and_paste: Fix bug where an unlabelled image could be pasted twice.

This commit removes the unnecessary condition for a linked image to have a label when checking whether to skip an image preview if its generating link has already been copied.

Screenshots and screen captures:
Composing:

Previewing:

Drafted:

Self-review checklist

• Self-reviewed the changes for clarity and maintainability
  (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

• Explains differences from previous plans (e.g., issue description).
• Highlights technical choices and bugs encountered.
• Calls out remaining decisions and concerns.
• Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

• Each commit is a coherent idea.
• Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

• Visual appearance of the changes.
• Responsiveness and internationalization.
• Strings and tooltips.
• End-to-end functionality of buttons, interactions and flows.
• Corner cases, error conditions, and easily imagined bugs.

[timabbott]

When writing tests https://spec.commonmark.org/0.18/#images, may be a good source of things to try. Since we're using upstream code, we may not find it easy to match CommonMark on everything, but it should be cheap to at least know where we're diverging.

[andersk]

The ![…](…) syntax indicates that the user knows this is an image. Not all images use an extension. We should give the user the image they asked for, not pretend we know “better”.

[N-Shar-ma]

I'd observed the convert-unsupported-image-to-link behavior here in GitHub actually, but I now understand the merits of letting it be as is, thank you!

[timabbott]

Can you give an example from GitHub of this detail?

[N-Shar-ma]

@timabbott On writing something like ![image](image.png) here in github, it renders as a plain link. Example:

[N-Shar-ma]

I've addressed most of the feedback above, most importantly moved the html wrangling for presentation of inline images to the frontend, but I'm still working on refining things further.

I'm yet to add backend tests, write a proper commit message, potentially split this across multiple commits, etc

[N-Shar-ma]

Have added tests and cleaned up the code and commit structure, this is ready for another review!

[N-Shar-ma]

When writing tests https://spec.commonmark.org/0.18/#images, may be a good source of things to try. Since we're using upstream code, we may not find it easy to match CommonMark on everything, but it should be cheap to at least know where we're diverging.

The reference style multiline syntax does not work, and any text inside [] is taken as is, so any inline syntax inside is copied to the image's alt attribute verbatim. These 2 are the main areas the implementation diverges from the spec. Other things like specifying the title attribute works as expected (see the added tests)

[N-Shar-ma]

@zulipbot add "chat.zulip.org. review"

[zulipbot]

ERROR: Label "chat.zulip.org. review" does not exist and was thus not added to this pull request.

[N-Shar-ma]

@zulipbot add "chat.zulip.org review"

[timabbott]

I merged the first two commits as the series ending with 633d814.

[timabbott]

So if you upload 3 images at once via dragging them or via selecting multiple files in the image picker, one gets two regression-looking outcomes:

• The images don't get put on the same line with each other, resulting in them stacking vertically rather than horizontally.
• The message controls seem to get displaced downwards in a message only containing such images.

[N-Shar-ma]

The images don't get put on the same line with each other, resulting in them stacking vertically rather than horizontally.

That is because we always put every upload (whether image or any other allowed format) in it's own line, so that's how they show up. What exact alternative behaviour are you proposing here @timabbott?

[alya]

Hm, yeah, the image arrangement might be OK?

[N-Shar-ma]

The message controls seem to get displaced downwards in a message only containing such images.

I'm not sure why exactly it is needed currently, but removing the baseline alignment for .messagebox-content introduced in this commit seems to fix this. @karlstolley would appreciate your insight here as the author of this commit!

[karlstolley]

That commit is necessary for the overall grid alignment in the message box. What I think needs to be applied here are the styles introduced in #26942 to the &:has(> .message_inline_image:first-child) selector to use the new paragraph-nested forms introduced here:

    &:has(> .message_inline_image:first-child),
    &:has(> p > .message_inline_image:first-child) {
        align-self: center;
    }

It might be possible to tidy up and simplify that selector, but we'd have to watch very carefully for similar regressions that that might introduce.

Also, it does appear in testing that the fallback work with @supports already addresses this case, so no further adjustment is needed there.

[timabbott]

For image arrangement, I think if you drag multiple images at once or select multiple in your browser in a single operation, it's probably more correct to put the images on a line together. Not sure if that's easy, but it was a major UX improvement when we made multiple linked images wrap on a single line (#20975), and this PR effectively undoes that feature from a practical standpoint.

[N-Shar-ma]

@karlstolley Thanks a lot for linking to that PR, I faintly remembered it but couldn't find it.

Since the problem was not for inline images that were 1st in their paragraph, but for any 1st paragraph containing an inline image, I've added the selector &:has(> p:first-child > .message_inline_image). I tested extensively manually and it seems to work fine with a variety of message compositions.:

[karlstolley]

If there's any text at all, that could give you a false negative. Test with just the image by itself, without any text. Your selector looks good to me, but I just want to make sure you're testing under the right/problem circumstances.

[N-Shar-ma]

I did test for that too, just didn't include it in the screenshot.

[N-Shar-ma]

@timabbott I've added a prep commit that puts all uploads made in one go on the same line. This means for all kinds of uploads, not just images, each link text will be inserted inline together all in 1 block, while the previews will be as before (horizontally stacked in another paragraph). For the new ![alt text](url) images, this means they too will stack horizontally when uploaded together.

Is this okay?

[timabbott]

Hmm, this might be fairly ugly in the case that you upload a mixture of previewable elements and non-previewable elements.

[N-Shar-ma]

@timabbott That's true ☹️ ... How about we only put them on 1 line if all the uploads are images (ie, anything we use the ![]() syntax for)?

[timabbott]

Yeah, that rule is probably a solid one.

[N-Shar-ma]

@timabbott Made the change! Now only for uploads with multiple image-only type files, are the files all added in their own line, so they render as being stacked horizontally

[zulipbot]

Heads up @N-Shar-ma, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.