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

Checkboxes support #681

Closed
giannissc opened this issue Dec 6, 2017 · 7 comments
Closed

Checkboxes support #681

giannissc opened this issue Dec 6, 2017 · 7 comments
Labels
bug Something isn't working duplicate feature request Request for a new feature

Comments

@giannissc
Copy link

I was under the impression that this was already submitted in the repository and would be included in the 1.1.0 update yet when i try to show a check box i only get this (image attached)

Steps to reproduce

  1. Create a check box list
  2. Press preview

What I expected

When writing check box list when pressing preview to render them properly

What happened instead

The check boxes were rendered as the equivalent HTML text for check boxes (see picture)

OS version

Ubuntu 17.10

Screenshot / Video

image

@dmsnell dmsnell added the bug Something isn't working label Dec 6, 2017
@laskin82
Copy link

same bug on windows 7

@jooji-san
Copy link

how can you create checkboxes? I'm typing [_] but it doesn't work

@laskin82
Copy link

laskin82 commented Jan 3, 2018

new note

- [] test1
- [] test1
- [] test1
- [] test1

@jooji-san
Copy link

thanks, but it doesn't work. shows me this in preview: <input type="checkbox" disabled style="margin: 0px 0.35em 0.25em -1.6em; vertical-align: middle;"> test1 <input type="checkbox" disabled style="margin: 0px 0.35em 0.25em -1.6em; vertical-align: middle;"> test1 <input type="checkbox" disabled style="margin: 0px 0.35em 0.25em -1.6em; vertical-align: middle;"> test1 <input type="checkbox" disabled style="margin: 0px 0.35em 0.25em -1.6em; vertical-align: middle;"> test1

@giannissc
Copy link
Author

giannissc commented Jan 4, 2018

It actually does work but it has a bug. Let me elaborate:

  1. You create a checkbox in markdown like this:
    - [] Blah blah
    - [x] Blah blah
  2. The markdown texted is parsed into the equivalent html
  3. The html text is parsed and rendered into the checkbox

Somehow step 3 does not happen and instead you only see the html text and not the checkbox that you are supposed to

@colinhowells
Copy link

One thing that'd be cool is if the 'preview' tab was interactive, so you could check the checkbox and when you went back to the 'edit' tab the checkbox'd be checked

@dmsnell
Copy link
Member

dmsnell commented Jan 5, 2018

Moving this into #314 where the feature request has been being tracked.

For the benefit of those in this thread I'll try and summarize why this isn't in yet:

  • Something is wrong on the render of the checkbox HTML as a result of some security sanitization going on. This is a bug and we just need to find and fix it.
  • Getting the checkbox to work from the preview is much harder than getting it to preview a checkbox. We had a working prototype in Add todo lists in the markdown preview #576 but that was only for the Electron app where the data flow is a bit simpler. We need to be able to get this out to the iOS, Android, macOS, and web versions too which sadly takes much more time.

@dmsnell dmsnell closed this as completed Jan 5, 2018
@dmsnell dmsnell added duplicate feature request Request for a new feature bug Something isn't working and removed bug Something isn't working labels Jan 5, 2018
dmsnell added a commit that referenced this issue Mar 10, 2018
See #681
See #694
See #721

The `showdown-xss` library wasn't really doing for us what we wanted.
It was transforming our checkbox inputs and making them display as plain
text instead of as actual HTML.

In this patch I've removed `showdown-xss` and created a new centralized
function to render HTML which calls our custom sanitization.

Why do it ourselves? We don't need the kind of sanitiation which only
removes the malicious code and leaves as much as it can untouched.
Instead we are free here to strip out basically everything except for
a few white-listed tags and attributes since we ourselves are the ones
producing the output; we don't have to support full HTML in the notes.

This patch should guard against everything on the OWASP list of XSS
attacks. It will remove significant styling and custom HTML but when a
tag is removed it will usually just take out the tag itself and leave
the inner content as plain text. Some tags are "forbidden" and all their
children are removed with them.
dmsnell added a commit that referenced this issue Mar 21, 2018
See #681
See #694
See #721

The `showdown-xss` library wasn't really doing for us what we wanted.
It was transforming our checkbox inputs and making them display as plain
text instead of as actual HTML.

In this patch I've removed `showdown-xss` and created a new centralized
function to render HTML which calls our custom sanitization.

Why do it ourselves? We don't need the kind of sanitiation which only
removes the malicious code and leaves as much as it can untouched.
Instead we are free here to strip out basically everything except for
a few white-listed tags and attributes since we ourselves are the ones
producing the output; we don't have to support full HTML in the notes.

This patch should guard against everything on the OWASP list of XSS
attacks. It will remove significant styling and custom HTML but when a
tag is removed it will usually just take out the tag itself and leave
the inner content as plain text. Some tags are "forbidden" and all their
children are removed with them.
dmsnell added a commit that referenced this issue Mar 22, 2018
* XSS Refactor: Replace `showdown-xss` with in-house sanitizer

See #681
See #694
See #721

The `showdown-xss` library wasn't really doing for us what we wanted.
It was transforming our checkbox inputs and making them display as plain
text instead of as actual HTML.

In this patch I've removed `showdown-xss` and created a new centralized
function to render HTML which calls our custom sanitization.

Why do it ourselves? We don't need the kind of sanitiation which only
removes the malicious code and leaves as much as it can untouched.
Instead we are free here to strip out basically everything except for
a few white-listed tags and attributes since we ourselves are the ones
producing the output; we don't have to support full HTML in the notes.

This patch should guard against everything on the OWASP list of XSS
attacks. It will remove significant styling and custom HTML but when a
tag is removed it will usually just take out the tag itself and leave
the inner content as plain text. Some tags are "forbidden" and all their
children are removed with them.

* also allow email links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

5 participants