Allow specific class values only #201
Conversation
| @@ -41,9 +41,7 @@ | |||
|
|
|||
| # Custom Additions | |||
| "*": ["id"], | |||
| "hr": ["class"], | |||
There was a problem hiding this comment.
| @@ -52,19 +50,43 @@ | |||
| "h4": ["align"], | |||
| "h5": ["align"], | |||
| "h6": ["align"], | |||
| "code": ["class"], | |||
There was a problem hiding this comment.
If we have a <code class=language-python>print("hello")</code> in the output of the Markdown rendering, we expressly change it to pygmentize before the bleaching is done, so at bleaching time we should not expect that tag.
| # Class is a specific attribute because not only do we want to allow it only | ||
| # on certain tags, but we also want to control possible values. | ||
| ALLOWED_CLASSES = { | ||
| "img": {"align-left", "align-right", "align-center"}, |
There was a problem hiding this comment.
If you look at https://github.com/pypa/warehouse/blob/master/warehouse/static/sass/blocks/_project-description.scss#L207-L209, you may find a discrepancy:
Warehouse CSS alignment rule will trigger on 3 possible conditions:
img.align-leftwhich is the one we target here.figure.align-leftwhich according to Styles for image alignment (pypa/warehouse#4023) pypi/warehouse#4391 (comment) will be applied on adivelement, but it's already broken as of today becausedivelement doesn't allowclasswithout our current bleaching rules, even before this PR. This could be fixed in a subsequent PRobject.align-leftbutobjecttags are not even allowed!
So authorizing only those classes and only onimgseems to not be a restriction compared to what we have today.
| "bp c c1 ch cm cp cpf cs dl err esc fm g gd ge gh gi go gp gr gs gt " | ||
| "gu il k kc kd kn kp kr kt l ld m mb mf mh mi mo n na nb nc nd ne nf " | ||
| "ni nl nn no nt nv nx o ow p py s s1 s2 sa sb sc sd se sh si sr ss sx " | ||
| "vc vg vi vm w x".split() |
There was a problem hiding this comment.
If you wonder, if pygmetize gets new CSS classes, it's not a problem that we won't have them here unless we want to style them in Warehouse, and in this case, we'll have to add them both here and in Warehouse CSS.
| @@ -9,7 +9,7 @@ commands = | |||
| extras = md | |||
|
|
|||
| [testenv:pep8] | |||
| basepython = python3.6 | |||
| basepython = python3 | |||
There was a problem hiding this comment.
The CI was launched using python 3.x but this tox target wanted 3.6 specifically, so it couldn't work. Don't know why it did so far. If you want this in a different PR, let me know.
|
The latest I've not studied the code here, but I did notice your earlier reference to the I'm sharing this in case there is any related concern to what you are working on here in this PR. |
|
Indeed! A good thing the dependency is restricted to <0.6.0 in setup.py, so our code won't have to handle both versions! I guess it's something we'll have to look at when we upgrade but hopefully, when we do this, the impacts will be studied. |
|
Oh there's an overlap with #120 ! |
In order to avoid strange things, it would be better to ensure that rendered READMEs only contain the subset of classes that we expect them to contain.