Support for Links in Polls, Issue: 12947 #16844
Conversation
|
It would be great if you could squash the commits. Especially the one regarding fixing lints and debugging. Refer https://zulip.readthedocs.io/en/latest/contributing/version-control.html for hints on how the commits need to be structured. |
zhark01
force-pushed
the
issue-12947
branch
3 times, most recently
from
03d88e6
to
44e9ed4
Compare
The change was made to support links in polls, as mentioned in issue zulip#12947. We used markdown renderer to render the link content, and parsed out any unnecessary p tags. We changed javascript and hbs files so that they properly render the content. Tested locally whether the links work, in addition to checking for XSS vulnerbilities. Everything tested worked, and no vulnerabilities discovered. Double check that there are no XSS issues. Fixes: zulip#12947
Add timeout to ensure there is enough time to render question through backend. There is no noticable lag for the user. Add coverage for submessage.
We have squashed the commits into only two, one refering to tests and one refering to widget features. I hope this looks better, let me know if there is any issues. |
| @@ -87,7 +86,6 @@ class PollData { | |||
| }; | |||
|
|
|||
| this.my_idx += 1; | |||
|
|
|||
There was a problem hiding this comment.
Can you remove these changes to existing code?
static/js/poll_widget.js
Outdated
| @@ -208,7 +206,7 @@ exports.activate = function (opts) { | |||
| const author_help = is_my_poll && !has_question; | |||
|
|
|||
| elem.find(".poll-question-header").toggle(!input_mode); | |||
| elem.find(".poll-question-header").text(question); | |||
| elem.find(".poll-question-header").html(question); | |||
There was a problem hiding this comment.
I'm concerned about the security implications of this.
|
Heads up @zhark01, we just merged some commits that conflict with the changes your 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 |
|
Thanks for working on this @zhark01! Closing this since the work is pretty incomplete; feel free to make a new PR addressing the feedback here if you want to get back to it. |
#12947
Testing plan:
Ran all of the tests in tools. Also, manually ran the server and tested numerous polls with links, both in question and in options. Tested for XSS vulnerabilities. Updated a few assert statements in poll_widget tests so that there is enough time for question to be rendered in the backend.