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] Promote URLs in plain text to HTML anchors after strip_tags. #5341

Merged
merged 10 commits into from
Sep 23, 2019

Conversation

tramuntanal
Copy link
Contributor

@tramuntanal tramuntanal commented Sep 18, 2019

🎩 What? Why?

Links in Proposal's body disappear.
I'm doing one thing in this PR:

  • converting URLs in plain text to anchors.

Discarded:

  • keeping existing html anchors, but sanitized

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests

📷 Screenshots (optional)

When entering "Content with <a href="http://urls.net" onmouseover="alert(document.cookies)">URLs</a> of anchor type and text urls like https://decidim.org." in the body of a Proposal, it gets rendered as follows:

image

@tramuntanal tramuntanal requested a review from a team as a code owner September 18, 2019 15:12
@tramuntanal tramuntanal self-assigned this Sep 18, 2019
@tramuntanal
Copy link
Contributor Author

@decidim/lot-core this PR is ready for review

tramuntanal and others added 3 commits September 19, 2019 13:00
Co-Authored-By: Oriol Gual <oriolgual@users.noreply.github.com>
[REFACTOR] Also added target blank and rel noopener.
@tramuntanal
Copy link
Contributor Author

tramuntanal commented Sep 19, 2019

@oriolgual @decidim/lot-core I've simplified this PR to simply converting plain text URLs to anchors.

Ready

Copy link
Contributor

@oriolgual oriolgual left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good enough for me, if in the future we need to add it to more places we can review the approach

oriolgual
oriolgual previously approved these changes Sep 20, 2019
@oriolgual oriolgual merged commit ae90815 into master Sep 23, 2019
@oriolgual oriolgual deleted the fix/links_in_proposals_body_disappear branch September 23, 2019 13:41
@oriolgual
Copy link
Contributor

oriolgual commented Sep 24, 2019

@tramuntanal I've deployed this to staging and it doesn't seem to work OK:

  1. When creating a proposal, the preview includes the HTML code in the text area

image

  1. The generated link doesn't have the target nor rel attributes:

http://staging.decidim.codegram.com/processes/itaque-voluptatem/f/3/proposals/43

@tramuntanal
Copy link
Contributor Author

Yes, I'll check!

oriolgual added a commit that referenced this pull request Sep 27, 2019
* Fix order to escape proposal content. Related to #5341

* Revert spec changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants