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

Reply box accepts rich text input #539

Closed
eloquence opened this issue Aug 20, 2019 · 4 comments · Fixed by #580
Closed

Reply box accepts rich text input #539

eloquence opened this issue Aug 20, 2019 · 4 comments · Fixed by #580
Labels
bug Something isn't working
Milestone

Comments

@eloquence
Copy link
Member

eloquence commented Aug 20, 2019

The client currently accepts rich text input in the reply box, e.g., via copy and paste:

Screenshot from 2019-08-19 23-48-26

While such input will be filtered when sending a reply, it should not be accepted at all, i.e. the reply box should act similar to an HTML <textarea>, treating all input as plain text.

@eloquence eloquence added the bug Something isn't working label Aug 20, 2019
@eloquence eloquence added this to the 0.2.0alpha milestone Aug 20, 2019
@deeplow
Copy link
Contributor

deeplow commented Oct 22, 2019

This might be simple fix: sed -i 's/QTextEdit/QPlainTextEdit/g' securedrop_client/gui/widgets.py

deeplow added a commit to deeplow/securedrop-client that referenced this issue Oct 22, 2019
reply box was accepting rich text input
@ninavizz
Copy link
Member

@eloquence Why shouldn't this accept rich text? I could see journalists liking this bug as a feature, tbh.

@eloquence
Copy link
Member Author

  • HTML is less safe to deal with than plain text and creates potential injection vectors for JavaScript or other malicious code. This is probably close to a dealbreaker issue for adding such functionality unless it's critically needed.
  • SecureDrop will not currently accept or render such replies and would need to be modified to do so (server-side change).
  • A rich-text control without any obvious way to disable or use it (e.g., toolbar) is of dubious value and more likely to cause random surprises when copying text from other contexts (exactly like the odd random icon in that screenshot).

@ninavizz
Copy link
Member

Cool, cool...

Your third point i was mostly curious about receiving feedback on, in the pilot; also somewhat expecting the user-surprise factor, but curious nonetheless. The first two points sound like clear dealbreakers to me, too, though... so, nevermind!

Ignore the ux kid waving her hands in the corner, and sortie-forth with debugging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants