Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix user-defined markup links targets #29305

Merged
merged 11 commits into from
Mar 8, 2024

Conversation

DanielMatiasCarvalho
Copy link
Contributor

@DanielMatiasCarvalho DanielMatiasCarvalho commented Feb 21, 2024

This seeks to fix the bug reported on issue #29196.

Cause:
ID's with custom characters (- , _ , etc.), were not linking correctly in the Markdown file when rendered in the browser because the ID in the respective destinies would be different than the one in anchor, while for IDs with only letters, the ID would be the same.

Fix:
It was suggested that to fix this bug, it should more or less like GitHub does it. While in gitea the anchors would be put in HTML like this:

<p dir="auto"><a href="#user-content-_toc152597800" rel="nofollow">Review</a></p>
<p dir="auto"><a href="#user-content-_toc152597802" rel="nofollow">Staging</a></p>
<p dir="auto"><a href="#user-content-_toc152597803" rel="nofollow">Development</a></p>
<p dir="auto"><a href="#user-content-_toc152597828" rel="nofollow">Testing</a></p>
<p dir="auto"><a href="#user-content-_toc152597829" rel="nofollow">Unit-tests</a></p>

In GitHub, the same anchor's href properties would be the same without "user-content-" trailing behind.

So my code made sure to change those anchors, so it would not include "user-content-" and then add respective Event Listeners so it would scroll into the supposed places.

Fixes: #29196

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 21, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 21, 2024
@silverwind
Copy link
Member

Oh and let's give the PR a proper description...

@silverwind silverwind changed the title Fix: Issue #29196 Fix pre-existing markup links href Feb 22, 2024
@silverwind silverwind changed the title Fix pre-existing markup links href Fix pre-existing markup links hrefs Feb 22, 2024
@lunny lunny added the backport/v1.21 This PR should be backported to Gitea 1.21 label Feb 26, 2024
@DanielMatiasCarvalho
Copy link
Contributor Author

@silverwind Do you think I need to add any more changes to this PR?

@silverwind
Copy link
Member

silverwind commented Mar 5, 2024

Did some style tweaks and added some explanatory comments to the file. Works as expected but I think above code block can be removed as we can just let those links work natively after the href is corrected. I also checked GitHub and they also don't bind any click on such links.

@silverwind silverwind changed the title Fix pre-existing markup links hrefs Fix user-defined markup links targets Mar 5, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 7, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 8, 2024
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 8, 2024
@silverwind silverwind enabled auto-merge (squash) March 8, 2024 09:40
@silverwind silverwind merged commit f219ea8 into go-gitea:main Mar 8, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 8, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 8, 2024
This seeks to fix the bug reported on issue go-gitea#29196. 

Cause: 
ID's with custom characters (- , _ , etc.), were not linking correctly
in the Markdown file when rendered in the browser because the ID in the
respective destinies would be different than the one in anchor, while
for IDs with only letters, the ID would be the same.

Fix:
It was suggested that to fix this bug, it should more or less like
GitHub does it. While in gitea the anchors would be put in HTML like
this:
```
<p dir="auto"><a href="#user-content-_toc152597800" rel="nofollow">Review</a></p>
<p dir="auto"><a href="#user-content-_toc152597802" rel="nofollow">Staging</a></p>
<p dir="auto"><a href="#user-content-_toc152597803" rel="nofollow">Development</a></p>
<p dir="auto"><a href="#user-content-_toc152597828" rel="nofollow">Testing</a></p>
<p dir="auto"><a href="#user-content-_toc152597829" rel="nofollow">Unit-tests</a></p>

```
In GitHub, the same anchor's href properties would be the same without
"user-content-" trailing behind.

So my code made sure to change those anchors, so it would not include
"user-content-" and then add respective Event Listeners so it would
scroll into the supposed places.

Fixes: go-gitea#29196

---------

Co-authored-by: silverwind <me@silverwind.io>
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 8, 2024
@lunny lunny modified the milestones: 1.23.0, 1.22.0 Mar 8, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 8, 2024
* giteaofficial/main:
  Add cache for branch divergence on branch list page (go-gitea#29577)
  Fix user-defined markup links targets (go-gitea#29305)
  Don't show AbortErrors on logout (go-gitea#29639)
  Style fomantic grey labels (go-gitea#29458)
  Don't use `<br />` in alert block (go-gitea#29650)
  Fix incorrect rendering csv file when file size is larger than UI.CSV.MaxFileSize (go-gitea#29653)
  Set user's 24h preference from their current OS locale (go-gitea#29651)
  Move get/set default branch from git package to gitrepo package to hide repopath (go-gitea#29126)
  Make runs-on support variable expression (go-gitea#29468)
silverwind added a commit that referenced this pull request Mar 8, 2024
Backport #29305 by @DanielMatiasCarvalho

This seeks to fix the bug reported on issue #29196. 

Cause: 
ID's with custom characters (- , _ , etc.), were not linking correctly
in the Markdown file when rendered in the browser because the ID in the
respective destinies would be different than the one in anchor, while
for IDs with only letters, the ID would be the same.

Fix:
It was suggested that to fix this bug, it should more or less like
GitHub does it. While in gitea the anchors would be put in HTML like
this:
```
<p dir="auto"><a href="#user-content-_toc152597800" rel="nofollow">Review</a></p>
<p dir="auto"><a href="#user-content-_toc152597802" rel="nofollow">Staging</a></p>
<p dir="auto"><a href="#user-content-_toc152597803" rel="nofollow">Development</a></p>
<p dir="auto"><a href="#user-content-_toc152597828" rel="nofollow">Testing</a></p>
<p dir="auto"><a href="#user-content-_toc152597829" rel="nofollow">Unit-tests</a></p>

```
In GitHub, the same anchor's href properties would be the same without
"user-content-" trailing behind.

So my code made sure to change those anchors, so it would not include
"user-content-" and then add respective Event Listeners so it would
scroll into the supposed places.

Fixes: #29196

Co-authored-by: DC <106393991+DanielMatiasCarvalho@users.noreply.github.com>
Co-authored-by: silverwind <me@silverwind.io>
silverwind added a commit that referenced this pull request Mar 8, 2024
Followup #29305. As per discussion
in #29666 (comment),
make this selector only search in the current `.markup` document, as
there can be multiples displayed at the same time.

@DanielMatiasCarvalho maybe you can review.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 8, 2024
Followup go-gitea#29305. As per discussion
in go-gitea#29666 (comment),
make this selector only search in the current `.markup` document, as
there can be multiples displayed at the same time.

@DanielMatiasCarvalho maybe you can review.
silverwind added a commit that referenced this pull request Mar 9, 2024
Backport #29679 by @silverwind

Followup #29305. As per discussion
in #29666 (comment),
make this selector only search in the current `.markup` document, as
there can be multiples displayed at the same time.

@DanielMatiasCarvalho maybe you can review.

Co-authored-by: silverwind <me@silverwind.io>
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Mar 13, 2024
Backport #29679 by @silverwind

Followup go-gitea/gitea#29305. As per discussion
in go-gitea/gitea#29666 (comment),
make this selector only search in the current `.markup` document, as
there can be multiples displayed at the same time.

@DanielMatiasCarvalho maybe you can review.

Co-authored-by: silverwind <me@silverwind.io>
(cherry picked from commit 25b0c99a41e9024d439deaa55be7ba87f6cd557f)
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Mar 13, 2024
Followup go-gitea/gitea#29305. As per discussion
in go-gitea/gitea#29666 (comment),
make this selector only search in the current `.markup` document, as
there can be multiples displayed at the same time.

@DanielMatiasCarvalho maybe you can review.

(cherry picked from commit baeb2511741aa70d24a48fd46db936b52be9d9dd)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
@DanielMatiasCarvalho DanielMatiasCarvalho deleted the issue-29196 branch March 21, 2024 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gitea 1.21.4 adds 'user-content-' in markdown file
5 participants