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

Use href of links instead of node.textContent for link previews #3871

Closed
mejo- opened this issue Mar 2, 2023 · 0 comments · Fixed by #3873
Closed

Use href of links instead of node.textContent for link previews #3871

mejo- opened this issue Mar 2, 2023 · 0 comments · Fixed by #3873
Assignees
Labels
bug Something isn't working high
Milestone

Comments

@mejo-
Copy link
Member

mejo- commented Mar 2, 2023

Describe the bug
Currently we use node.textContent to determine whether a paragraph is a link that warrants a link preview. Instead, we should check whether the paragraph has a link mark and use its href.

Reasons for this change would be:

  • We would make sure to always show the preview of the link target, not of the description text. Both may differ, which has security implications.
  • Links with a custom description would get a link preview as well.
  • Apparently url-encoded spaces in textContent of links get decoded when transformed to markdown. Spaces are not regarded in the regex that checks for URLs. Therefore URLs with spaces loose their link preview once the Text session is closed.

I'll take a look into implementing this change.

@julien-nc would be interested in your opinion on that.

@mejo- mejo- added bug Something isn't working high labels Mar 2, 2023
@mejo- mejo- added this to the Nextcloud 26 milestone Mar 2, 2023
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Mar 2, 2023
@mejo- mejo- moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Office team Mar 2, 2023
@mejo- mejo- self-assigned this Mar 2, 2023
@mejo- mejo- changed the title Use href of links instead of node.textContent for link previews instead of Use href of links instead of node.textContent for link previews Mar 2, 2023
mejo- added a commit that referenced this issue Mar 2, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit that referenced this issue Mar 3, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit that referenced this issue Mar 3, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
@github-project-automation github-project-automation bot moved this from 📄 To do (~10 entries) to ☑️ Done in 📝 Office team Mar 3, 2023
backportbot-nextcloud bot pushed a commit that referenced this issue Mar 3, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
backportbot-nextcloud bot pushed a commit that referenced this issue Mar 3, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit that referenced this issue Mar 5, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit that referenced this issue Mar 5, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit that referenced this issue Mar 8, 2023
Until now we used node.textContent to determine whether a paragraph is
a link that warrants a link preview. Instead, we now check whether the
paragraph has a single text node wich is a link and use its href.

Text nodes with empty textContent are ignored in order to allow
whitespaces before and after the link.

This way we ensure to always show the preview of the link target, not of
the description text. Both may differ, which has security implications.

Also, links with a custom description get a link preview as well.

And last but not least, it fixes link previes for URLs with spaces.
(Background: for some reason, url-encoded spaces in textContent of links
get decoded when they're transformed to markdown and written to a file.
Therefore URLs with spaces lost their link preview once the Text session
was closed prior to this commit)

Fixes: #3871

Signed-off-by: Jonas <jonas@freesources.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant