Shalitha Suranga wants to merge 12 commits from /u/shalithasuranga/allura/ to master, 2018-09-14
I have added checklist support [#8085] and these files are changed
requirements.txt - added markdown checklist extension
app_globals.py - added markdown extension to ext array
How sub issues were solved
short reference links are affecting and instead checklist there were links. Thus I modified
markdownextensions.py's ForgeLink Pattern
checkbox was not rendered to do sanitize function. Therefore I commented input element from blacklist of util.py
Please review and suggest your ideas
Commit | Date | |
---|---|---|
2018-09-11 17:02:56 | Tree | |
2018-09-11 16:47:04 | Tree | |
2018-09-10 17:07:39 | Tree | |
2018-09-10 17:06:31 | Tree | |
2018-09-10 15:33:52 | Tree | |
2018-09-10 15:32:11 | Tree | |
2018-09-06 17:24:59 | Tree | |
2018-09-05 16:37:39 | Tree | |
2018-09-02 14:12:23 | Tree | |
2018-09-02 14:11:46 | Tree | |
2018-09-02 14:08:58 | Tree | |
2018-09-02 14:04:21 | Tree |
Hi, thanks for this! It's pretty nice improvement. As a new feature though, there is some improvements & changes to do:
It would be good to add a test for this specifically.
The
allura.tests.test_utils.TestHTMLSanitizer.test_html_sanitizer_form_elements
test is failing. You could just update the test to reflect the fact that<input>
tags are displayed now. But I think it would be best of only the checkboxes were allowed and other input types were still sanitized. I think you could do this by uncommenting(ns_html, 'input'),
again and then indef sanitize_token
after it does some very specific checking for iframes, add some specific checking for input tags. I'm thinking if it is aninput
andtype
ischeckbox
then allow it, but don't allow other types (text, radio, submit, etc). And it's just a single tag so that should make the code simpler than the iframe check (you won't have to deal with "EndTag" and tracking anything like_prev_token_was_ok_iframe
hopefully)if(el.text == 'x' or el.text == ' '):
doesn't need the parenthesis - that will make it look a little nicer and match PEP8 guidelines. Also I would suggest adding a comment here. So someone later can know that these lines are for checklists to work (otherwise its not very obvious what they are doing).The markdown help page should be updated to mention this new feature.
Have you looked into making the checkboxes interactive if you have edit permissions? It looks like the markdown-checklist package has some helper JS for it, but would take custom code to handle the ajax and do the update. Don't to do this now, but it would be nice later of course.
Hi... Dave
Thank you for the guidance. Yeah it was easy with sanitize token. I did those changes now..I will push commits by adding unit test for this improvement
Hi.. Dave
I have made these changes and pushed to marge request
allura.tests.test_utils.TestHTMLSanitizer.test_html_sanitizer_checkbox
Please review and advice next steps
Thanks
Looks good, thanks!
Thanks for merging .. welcome