Open Zren opened 10 years ago
Martii
commented
10 years ago Make sure we triple check the html markup sanitizing and md (gfm)... there's currently an issue with content in code fences being stripped nicely... haven't reported it yet because everyone can work around it with <pre><code> tags and I am not entirely sure when it appeared.
One exampled dev/production homepage for my display of the sample json.
See also:
sizzlemctwizzle
commented
10 years ago there's currently an issue with content in code fences being stripped nicely... One exampled homepage for my display of the sample json
Maybe I need to get my eyes checked, but what exactly is wrong with that json? What do you mean by "stripped nicely"?
sizzlemctwizzle
commented
10 years ago @Zren
Have you seen @jerone's script where he adds a bunch of other buttons for GitHub. I'm guessing that it probably can't be easily adapted to Bootstrap Markdown, but it's a good example of what buttons it would be nice to have.
Martii
commented
10 years ago but what exactly is wrong with that json?
Should look like this:
{
"Monkey Barrel": [
"/scripts/show/114843",
{
"recent posts": "/posts?kind=all",
"recent topics": "/topics",
"recent comments": "/comments",
"recent images": "/images",
"": "",
"recent spam": "/spam",
"recent potential spam": "/posts?kind=all&spam=1",
"recent potential spam by score": "/posts?kind=all&spam=score",
"spam and malware \u00bb": "/topics/9#posts-last",
"cookie stealing scripts \u00bb": "/topics/704#posts-last"
}
],
"Scripts": [
"/scripts",
{
"recent posts": "/posts?kind=script",
"recent reviews": "/reviews",
"recent potential spam": "/posts?kind=script&spam=1",
"recent potential spam by score": "/posts?kind=script&spam=score",
"": "",
"popular scripts": "/",
"all scripts by name": "/scripts?sort=name",
"most discussed": "/scripts?sort=posts",
"highest rated": "/scripts?sort=rating",
"most favorited": "/scripts?sort=fans",
"most installed": "/scripts?sort=installs",
" ": "",
"new script by upload": "/scripts/new",
"new script by pasting": "/scripts/new?form=true"
}
],
"Tags": [
"/tags",
{
"most frequently used tags": "/tags?sort=count",
"all tags by name": "/tags?page=1"
}
],
"Forums": [
"/forums",
{
"recent posts": "/posts?kind=forum",
"recent potential spam": "/posts?spam=1",
"recent potential spam by score": "/posts?kind=forum&spam=score",
"": "",
"script development": "/forums/1",
"ideas and script requests": "/forums/2",
"userscripts.org discussion": "/forums/3",
"the banana bar": "/forums/4",
"greasefire": "/forums/5"
}
],
"People": [
"/users",
{
"all members by name": "/users?page=1",
"all members by # of scripts": "/users?sort=scripts",
"all members by # of comments": "/users?sort=comments",
"all members by # of posts": "/users?sort=posts"
}
],
"Blog": [
"/articles",
{
"rss feed": "/feeds/recent_articles"
}
],
"Guides": [
"/guides",
{
"highest rated": "/guides?sort=votes",
"most discussed": "/guides?sort=comments",
"sorted by author": "/guides?sort=author",
"sorted by date": "/guides?sort=updated"
}
]
}but looks like this:
{
"": ,
"": ,
"": ,
"": ,
"": ,
"": ,
"": }
Zren
commented
10 years ago Hmmm. It stores fine, it must be somewhere in the rendering.
https://openuserjs.org/admin/json?model=Script&id=537812c3fb78e9000055d860
Examples\r\n\r\n``` json\r\n{\r\n \"Monkey Barrel\": [\r\n \"/scripts/show/114843\",\r\n {\r\n \"recent posts\": \"/posts?kind=all\",\r\n \"recent topics\": \"/topics\",\r\n \"recent comments\": \"/comments\",\r\n \"recent images\": \"/images\",\r\n \"\": \"\",\r\n \"recent spam\": \"/spam\",\r\n \"recent potential spam\": \"/posts?kind=all&spam=1\",\r\n \"recent potential spam by score\": \"/posts?kind=all&spam=score\",\r\n \"spam and malware \\u00bb\": \"/topics/9#posts-last\",\r\n \"cookie stealing scripts \\u00bb\": \"/topics/704#posts-last\"\r\n }\r\n ],\r\n \"Scripts\": [\r\n \"/scripts\",\r\n {\r\n \"recent posts\": \"/posts?kind=script\",\r\n \"recent reviews\": \"/reviews\",\r\n \"recent potential spam\": \"/posts?kind=script&spam=1\",\r\n \"recent potential spam by score\": \"/posts?kind=script&spam=score\",\r\n \"\": \"\",\r\n \"popular scripts\": \"/\",\r\n \"all scripts by name\": \"/scripts?sort=name\",\r\n \"most discussed\": \"/scripts?sort=posts\",\r\n \"highest rated\": \"/scripts?sort=rating\",\r\n \"most favorited\": \"/scripts?sort=fans\",\r\n \"most installed\": \"/scripts?sort=installs\",\r\n \" \": \"\",\r\n \"new script by upload\": \"/scripts/new\",\r\n \"new script by pasting\": \"/scripts/new?form=true\"\r\n }\r\n ],\r\n \"Tags\": [\r\n \"/tags\",\r\n {\r\n \"most frequently used tags\": \"/tags?sort=count\",\r\n \"all tags by name\": \"/tags?page=1\"\r\n }\r\n ],\r\n \"Forums\": [\r\n \"/forums\",\r\n {\r\n \"recent posts\": \"/posts?kind=forum\",\r\n \"recent potential spam\": \"/posts?spam=1\",\r\n \"recent potential spam by score\": \"/posts?kind=forum&spam=score\",\r\n \"\": \"\",\r\n \"script development\": \"/forums/1\",\r\n \"ideas and script requests\": \"/forums/2\",\r\n \"userscripts.org discussion\": \"/forums/3\",\r\n \"the banana bar\": \"/forums/4\",\r\n \"greasefire\": \"/forums/5\"\r\n }\r\n ],\r\n \"People\": [\r\n \"/users\",\r\n {\r\n \"all members by name\": \"/users?page=1\",\r\n \"all members by # of scripts\": \"/users?sort=scripts\",\r\n \"all members by # of comments\": \"/users?sort=comments\",\r\n \"all members by # of posts\": \"/users?sort=posts\"\r\n }\r\n ],\r\n \"Blog\": [\r\n \"/articles\",\r\n {\r\n \"rss feed\": \"/feeds/recent_articles\"\r\n }\r\n ],\r\n \"Guides\": [\r\n \"/guides\",\r\n {\r\n \"highest rated\": \"/guides?sort=votes\",\r\n \"most discussed\": \"/guides?sort=comments\",\r\n \"sorted by author\": \"/guides?sort=author\",\r\n \"sorted by date\": \"/guides?sort=updated\"\r\n }\r\n ]\r\n}\r\n```
Edit: Er, of course it stores fine. We only sanitize/render when it gets parsed. Not sure why I was thinking the MD Editor would strip it out.
sizzlemctwizzle
commented
10 years ago Stripping is done by xss. Removing that call shows the json properly.
sizzlemctwizzle
commented
10 years ago Okay, the problem is that highlight.js is generating class attributes to style the code and that is not a default whitelisted attribute, so the tags (and their contents) get removed.
sizzlemctwizzle
commented
10 years ago Would not stripping the class attribute be horrible?
Martii
commented
10 years ago Would not stripping the
classattribute be horrible?
You mean whitelisting the class attribute? It all depends on if javascript:void(0) and everything else script related in the value would be able to run for XSS attacks. I don't think I've ever tried that... most, if not all, of CSS rules are for styling only... so any risk would probably be in changing the style for that particular element in the user-content type areas... I'm not 100% sure because it's a use case I've never thought of. Might be a handy thing for allowing some common or user account defined classes down the line.
USO doesn't allow the class attribute yet their md sort of works... they also have code fence issues... which is a reason why I usually never use md there ... I would like to encourage use of github-flavored-markdown (gfm) though over markup... but as we've chatted about in the past some things md just can't do.
Have you tried doing your original commit but whitelist class ? to see if https://github.com/OpenUserJs/OpenUserJS.org/issues/125#issuecomment-44907472 goes away? Just a random thought and will probably not work... but worth a quick try.
This might be a bug in simple-xss not recognizing that content between properly terminated<pre> and <code> tags should be ignored too or even md aware. IDK... or reversing the logic of ignoring ` and ```. < --- see even GH has issues going one way ... supposed to be visualized as being described as the single backward apostrophe encased in code tags to denote code tag and three backward apostrophes for code fencing.
See also:
Martii
commented
10 years ago NOTE: More links added in previous post to examine
Going to do some tests here... so this particular reply may change some:
Define a table with GH gfm
| Left | Center | Right |
|---|---|---|
| abcdefghijklmnopqrstuvwxyz | a | abcdefghijklmnopqrstuvwxyz |
Test to determine if GH gfm has a class attribute to center the middle column..... NO it uses the align attribute... this is different than our gfm implementation I think... this result kills further testing for trying to apply the class value into another markup tag to see if GH allows class hijacking. :\
Btw if you didn't notice my Compatibility Matrix is also sanitized on OUJS now... most of my icons groupings are supposed to be centered with the class that is added via :----: and is stripped. (I used <center> on USO)
Swipe the GH generated output table source and repost as markup
| Left | Center | Right |
|---|---|---|
| abcdefghijklmnopqrstuvwxyz | a | abcdefghijklmnopqrstuvwxyz |
... result: ALLOWS align attribute
NOTE: align attribute is not HTML5 compliant and recommendations for using CSS are in quite a few places.
This p tag has classy as a class
.... result: SANITIZED
Applying the `align` attribute for `p` tag.. should be far `right`
... result: SUCCESS with a consequence of GH gfm FAILING
Martii
commented
9 years ago So bringing this back to topic basically we're missing "pin" which I don't recommend keeping our comment reply box open constantly. It has full screen editing now which is pretty good.
The other thing I see @jerone 's script is horizontal rule e.g. ---... is this really a necessity? There's a vague reference to "additionalButtons" over on that project under the API header (sorry no direct linkage because no id or name)
What buttons does anyone want to see?
I would love to see a "Reply and Close" button on Issue discussions if possible... any thoughts?
Martii
commented
8 years ago What buttons does anyone want to see?
Reiterating again... will close soon if there isn't any interest.
Martii
commented
6 years ago Investigated this. bootstrap-markdown only supports jQuery $('selector').markdown({ additionalButtons...
Due to migration away from jQuery in our core, esp. in the client-side scripts, this is now not programmatically an option. e.g. Does not currently work with node.querySelector or the like. Ref: toopay/bootstrap-markdown/blob/878f349/js/bootstrap-markdown.js#L61
We could however fork the project and hard code the needed buttons. We would need to maintain this with all upstream changes.
Currently strike is missing and GFM tables... both of which are specific to GFM. CommonMark may have something different available but it's still newer to the scene. It is possible to append those using jQuery and their API however it's quite messy to get the buttons in the right order.
But I'm going to close this very soon due to complete lack of interest shown. I really think there are plenty of buttons already.
Martii
commented
6 years ago So looked into Jerones script source... he mixes markup with markdown. e.g. <ins></ins> for underline. He also supports GH task lists, multiple header values, tab insertion, UserAgent insertion, Horizontal Rule, selection delete, emojis, notice for CONTRIBUTING.md., GFM tables, etc. Bad news is that script doesn't seem functional anymore (deprecation at https://github.com/jerone/UserScripts/issues/106#issuecomment-244619958) and screenshots outdated which is why the source needed to be examined.
The more I think of this the better it would be to do this .user.js client-side if more are wanted. Dabbling on a local copy of bootstrap-markdown this can be achieved directly, and in jQuery currently, but again order is the thing when just adding buttons the "normal way" (via that maintainers API).
I'm at a -1 for modifying bootstrap-markdown... way too messy.
Btw last chance to convince otherwise.
https://openuserjs.org/discuss/The_new_layout
https://toopay.github.io/bootstrap-markdown/ https://github.com/toopay/bootstrap-markdown/blob/master/js/bootstrap-markdown.js#L750