Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Markdown image paste regressions #31230

Closed
wxiaoguang opened this issue Jun 3, 2024 · 7 comments · Fixed by #31235
Closed

Markdown image paste regressions #31230

wxiaoguang opened this issue Jun 3, 2024 · 7 comments · Fixed by #31235
Labels
issue/regression Indicates a previously functioning feature or behavior that has broken or regressed after a change
Milestone

Comments

@wxiaoguang
Copy link
Contributor

Regression of Downscale pasted PNG images based on metadata #29123 :

  1. The pasted image is not clickable (unable to view the full image)
  2. The URL path doesn't contain sub-path if the Gitea instance is deployed under a sub-path.
    • From discord report: Pasting image from the clipboard the Javascript (Paste.js) embeds the image via html code but the src attribute is set to value: /attachments/<GUID>. Using a configuration in which the Gitea application is served on a subpath of the domain name, for example: https://domain.name/git the image is not rendered.
@wxiaoguang
Copy link
Contributor Author

@silverwind

@lunny lunny added the issue/regression Indicates a previously functioning feature or behavior that has broken or regressed after a change label Jun 3, 2024
@lunny lunny added this to the 1.22.1 milestone Jun 3, 2024
@silverwind
Copy link
Member

silverwind commented Jun 3, 2024

Both issues indicate there is some processing that works on ![]() images, but not on the <img> variant, so it's not a direct regression from that PR because <img> is supported since a long time, it just was never noticed.

@silverwind
Copy link
Member

silverwind commented Jun 3, 2024

Can confirm above suspicion, there is transformImage which apparently is never applied on user-supplied <img> tags because maybe Goldmark does not emit it as ast.Image or something.

@silverwind
Copy link
Member

silverwind commented Jun 3, 2024

Yeah so this is no easy fix in backend because <img> tags do not go through goldmark AST processing I think, they likely go through as raw html so need to introduce the wrapper link here:

if node.Data == "img" {

@wxiaoguang
Copy link
Contributor Author

Actually it is a regression. No matter how markdown / img work, as an end users:

  1. I would like to click an image to show the full size (especially when it is a full-size screenshot)
  2. I would like to deploy Gitea under a sub-path and paste images into the editor

I do not care about <img> or something else, but now:

  1. I can't
  2. I can't

As a developer, I think there is an undefined / unclear behavior for the embedded <img> tag when it has a relative src link. But I do not think it's worth to make it more complex than it should be at the moment.

@wxiaoguang
Copy link
Contributor Author

-> Make pasted "img" tag has the same behavior as markdown image #31235

@silverwind
Copy link
Member

I would like to click an image to show the full size (especially when it is a full-size screenshot)

At least Firefox has the built-in on all images "Open image in new tab" 😉

lafriks pushed a commit that referenced this issue Jun 4, 2024
Fix #31230

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jun 4, 2024
wxiaoguang added a commit that referenced this issue Jun 4, 2024
#31243)

Backport #31235 by wxiaoguang

Fix #31230

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 2, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
issue/regression Indicates a previously functioning feature or behavior that has broken or regressed after a change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants