Talk:Requests for comment/Allow styling in templates

Yes
Raise your hands here:


 * +1 Jdlrobson (talk) 18:43, 13 June 2013 (UTC) This would reduce the need for a lot of hacks that currently exist in the MobileFrontend to workaround this problem and usually don't do very well at it.
 * +1 CSteipp (talk) 17:28, 19 June 2013 (UTC) In the future, I would like to reach the point where we can have a CSP rule that does not include 'unsafe-inline' for styles. I see this as a first step in that direction.
 * Tfinc (talk) 23:56, 22 August 2013 (UTC)
 * +1 Cscott (talk) 18:06, 26 August 2013 (UTC)

No
Wave your angry fists here:

For use of a Style tag
Sign here:
 * This allows to include it in conditional #if’s and to be able to dynamically change some values (e.g. width of some elements, etc.). I’m not sure it is really needed and useful and good programming, but I just mention it. ~ Seb35 09:18, 22 July 2013 (UTC) By re-reading it, I think the CSS in this category should not be "dynamic", and must be sanitised (e.g. no &lt;includeonly>, &lt;translate>, or other wikitext-special tags must be here). Even if the sanitisation here is more difficult, it would be more coherent for the template: same history to track the changes, easier exporting (if any central repository of templates), same permissions. ~ Seb35 [^_^]  09:16, 30 December 2013 (UTC)
 * Seems more consistent with the current content model and security policy. Separate CSS requires more thorough sanitization of, eg, @import statements, external links, etc. Cscott (talk) 18:07, 26 August 2013 (UTC)

For use of a separate css file
Shout here:
 * +1 Jdlrobson (talk) 18:43, 13 June 2013 (UTC) I would personally like to see and implementation this way as it allows us to easily separate the entire css into a separate file in the future and decouples the CSS from the wikitext. Templates are already scary enough as it is without style tags! :)
 * +1 S Page (WMF) (talk) 01:50, 3 July 2013 (UTC) Yes I can't see a benefit to a style tag. One file seems simpler at first but separation of concerns makes it easier to work separately. Plus syntax highlighters and checkers can do a better job. It would be great if the CSS file allowed  so users could pull in kewl CSS, and for super-bonus points if MW could figure out a "What links here" for CSS files based on this (ContentHandler can do anything, right?)
 * Note if LESS does get into core templates could potentially support LESS which should give lots of power :) Jdlrobson (talk) 18:33, 5 September 2013 (UTC)
 * +1 We have a beautiful resource loading system. Minification, concatenation, RTL support, etc... It would be nice if template styles could make use of this by being loaded from a separate page (after sanitization). Daniel Friesen (Dantman) (talk) 22:54, 11 September 2013 (UTC)

Selected trusted group

 * +1 I think editing css requires a certain level of trust and knowledge. In a similar way to how we only allow certain editors to edit MediaWiki:Common.css I'd assume the same to apply to template css. Allowing anyone to edit CSS regardless of experience is asking for trouble in my opinion - it's important to keep the frontend of the site as efficient as possible - https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Writing_efficient_CSS. Jdlrobson (talk) 19:21, 13 June 2013 (UTC)

Same permissions as accompanying page (template)

 * +1 Both regular templates (ParserFunctions) and ones using Lua require knowledge for many things (though there is still sometimes room to make copy changes without such expertise). Mistakes or bad code can have serious performance ramifications (generally far worse than bad CSS would cause).  However, the fact that templates are open by default means new people are capable of gaining this knowledge, and improvements can be made faster.  Commonly used templates are already protected on many wikis.  Given that templates and their CSS will generally be edited in conjunction, I think we should link the protection, and continue deferring to the community's judgement about which templates should be protected, and at what level. Superm401 - Talk 21:33, 13 June 2013 (UTC)
 * The issues with CSS are not performance related they're security related. Daniel Friesen (Dantman) (talk) 21:57, 13 June 2013 (UTC)
 * I agree (although it's both, performance is the lesser consideration). As I said on Wikitech, there needs to be a sanitizer.  In addition to that, I've suggested below scoping it to the body to prevent people from trying to style interface elements (the sanitizer is still needed to make sure they don't try absolute positioning tricks, etc.). Superm401 - Talk 22:01, 13 June 2013 (UTC)
 * +1 Cscott (talk) 18:08, 26 August 2013 (UTC)
 * +1 (and I’m in favour of a &lt;style> inside the template) ~ Seb35 [^_^]  09:21, 30 December 2013 (UTC)

What measures need to be made to take into account performance and security
Please voice any concerns you have here:


 * Template css would have to be restricted to a certain audience Jdlrobson (talk) 18:43, 13 June 2013 (UTC)
 * Template CSS should be properly sanitized, and scoped to only affect the content. Ideally, it would only affect the output of the template itself, but I'm not sure how to implement that, given that templates can use different element types, some only open a tag, some only close, some do neither, etc.  So I suggest scoping it to bodyContent or something narrow (possibly skin-dependent) containing only the body.  There should also be an autogenerated comment at the top of the CSS so it's easy to track down what template is responsible for certain code.  So if someone writes the below in Template:Columns.css:

it will be transformed to:

It does lengthen the selectors, and it there could still be leakage to other parts of the content. However, it shouldn't affect interface elements, and the comment (which needs to survive RL) would allow tracking down the offending template pretty easily. Superm401 - Talk 21:57, 13 June 2013 (UTC)


 * As mentioned by Tim on, "Sanitizer::checkCss only validates an individual declaration, there may be new security issues in the validation of entire stylesheets.". So we do need to work out what the css parser would look like, and what would be allowed from an arbitrary blob of css. Possibly use HTMLPurifier's css sanitization? CSteipp (talk) 17:02, 27 August 2013 (UTC)


 * Cache : Add the CSS/JS in the templates could fragment the client cache (I not really knowledgeable about this and the way RessourceLoader handles this, I may be wrong). It is probably better for the global load that the CSS/JS of a rarely-used customised template is loaded in a separated page than the global CSS/JS, but on the other side it is better to load the CSS/JS of a frequently-used customised template in the global CSS/JS to avoid loading it in a separated CSS/JS call. Probably the better solution to this is to either have a checkbox to let the user decide if the CSS/JS should be included in the global or "local" CSS/JS, either heuristically determine it given the statistics of use of the template. ~ Seb35 09:46, 22 July 2013 (UTC)


 * Tracking of the changes : In comparison with the collect of all CSS rules/JS functions in a global CSS/JS page, such an editing model of the CSS/JS would decrease the central management of the CSS/JS done by trusted users (administrators) and this could lead to fears from administrators and regular users (CSS/JS hacks of templates with repercussions on all pages where the template is transcluded). Some listing of the customisations on the templates could help handle the vandalism operated through CSS/JS; this would be quite immediate if the CSS/JS are on a separated associated page (e.g. Template:Name/css) with the possibility of watching the pages and with the "trailer" param in Special:Recentchanges (although it seems it only work on translatewiki with /langcode). ~ Seb35 11:29, 22 July 2013 (UTC) If it is chosen to have &lt;style> tag instead of a separated page, &lt;style>-customised templates could be added in a category in order to follow these with Recentchangeslinked. ~ Seb35 [^_^]  09:35, 30 December 2013 (UTC)


 * 1) bodyContent is skin-specific. Use #mw-content-text. Daniel Friesen (Dantman) (talk) 22:41, 11 September 2013 (UTC)

JavaScript
The RFC focused here on CSS could also be easily extended to the JavaScript associated with some templates (collapsible tables, fr:Template:Images, fr:Template:Titre incorrect, en:Template:Link FA, etc.). Possibly the response is different (security mainly) but the general problem is the same. ~ Seb35 10:02, 22 July 2013 (UTC)

Consistency
This would add a bit of consistency in the tracking of the changes of a specific template (no need to check the Common.js/css in addition of the template page), would ease the copy of the templates accross wikis and would be in the good direction in a global future goal of sharing some templates accross a set of wikis. ~ Seb35 10:38, 22 July 2013 (UTC)

Scoped CSS
I think this could be implemented by simply implementing the scoped CSS extension (using polyfills for older browsers, such as https://github.com/PM5544/scoped-polyfill). Unbalanced templates would allow the CSS to 'leak', but not out of the content area, and that's no different from any of the other things unbalanced templates can do. It's an orthogonal issue. Cscott (talk) 18:10, 26 August 2013 (UTC)


 * I should note that simply using scoped css alone won't prevent css from applying to stuff outside of the scope if someone malicious wants it to. Even with a polyfil to handle browsers that don't support scoped css tags the standard for scoped css permits rules prefixed with  to apply outside the scope. Daniel Friesen (Dantman) (talk) 22:51, 11 September 2013 (UTC)

Bugzilla
This also seems to be bug 35704. Discussion should probably happen here, I'm just pointing out the link. Cscott (talk) 18:13, 26 August 2013 (UTC)

Class-triggered CSS includes
An alternative idea is to use CSS classes with a mediawiki-specific prefix (mw-* for example) for content styling. Based on those prefixed classes, CSS fragments can be pulled from a CSS namespace (mw-foo -> CSS:foo), and assembled into a block of CSS in the head. Multiple rules and media selectors can be defined for each class-triggered CSS include. Scoping can be enforced by prefixing each rule with the class selector (.mw-foo). The assembly of the content CSS should be done in alphabetical order to ensure deterministic precedence.

Advantages:
 * works for both page and template content
 * no issues with unbalanced transclusions
 * shared definitions for small network transfers
 * no immediate need for scoped style element support in the browser

Performance should be good if the list of classes and possibly the CSS itself is generated on save. Similar to template links, pages using a class should probably be tracked with a classlinks table, so that caches can be updated and the consequences of a potential edit can be considered. -- Gabriel Wicke (GWicke) (talk) 00:01, 13 September 2013 (UTC)
 * is reserved for core and extension code, so we would need a different prefix. Beyond that, this is interesting, but there are limitations.  It's not obvious how to do something (or where it should go) like:


 * -- Superm401 - Talk 22:58, 16 September 2013 (UTC)


 * The media query should work as expected when placed in CSS:some-navbox. That page's full CSS content including the media query would be included whenever class="mwc-some-navbox" (assuming a prefix of ) is encountered. Can you describe the potential issue you see? -- Gabriel Wicke (GWicke) (talk) 05:35, 17 September 2013 (UTC)


 * This is definitely interesting and much more flexible than my original suggestion. The thing I like most about it is that it allows sharing of stylesheets across templates which is even better. How would I register a stylesheet would it be  or would it simply deduce them from the classes present? Jdlrobson (talk) 18:36, 4 December 2013 (UTC)


 * Using a class with the specific prefix in content would trigger the inclusion. To make this efficient the list of used content classes would be saved in separate metadata at parse time. -- Gabriel Wicke (GWicke) (talk) 23:50, 4 December 2013 (UTC)