Clean name-spaces for CSS for 0.11+?

[shap@…]

I suspect that it may be too late for this, but I think this is a low-risk change.

Trac uses CSS all over the place. This is good. The problem is that I need to pass through some content that *also* uses CSS, and there is a problem of name space collision in the choices of names for the class attribute. Since Trac is cutting over the template system, now is an opportune time to look at CSS issues if there is a desire to do so.

How difficult would it be for Trac to adopt the convention that all class attributes and all id attributes that are emitted from the trac templates would begin with "trac-" so that they are less likely to collide with CSS within content?

In our case, we cannot simply import the incoming content once and have done; it is a reference manual that we need to maintain externally via our version control system. Our goal is to simultaneously make it available within the Wiki for commenting and feedback. I considered re-writing the class attributes on our content using XSLT. It is doable, but it is probably beyond the skills of most XSLT authors.

[Emmanuel Blot]

See also #4264.

[Remy Blank]

#7932 reported a possibly frequent clash with Trac element ids (a heading like === attachments ===, which gets an attribute id="attachments", so the CSS for the attachments <div> matches), which would be solved by a clean namespace for element ids. It was closed as a duplicate.

Prefixing all element ids with e.g. trac- should not be too difficult, as ids are mostly referenced by the style sheets. However, IIRC, element classes are quite often referenced in the code, which would make the change much more painful.

I would like to at least start the process and fix the element ids. Would trac- be a good prefix?

[Emmanuel Blot]

Replying to rblank:

I would like to at least start the process and fix the element ids. Would trac- be a good prefix?

The trouble is that this will break all the customized Trac installations. I guess there are many of them… So either this replacement has to wait for a major release (0.12) or better, provide a Python script to parse and update custom CSS file to match the new naming scheme.

[Remy Blank]

Replying to eblot:

The trouble is that this will break all the customized Trac installations. I guess there are many of them… So either this replacement has to wait for a major release (0.12)

That was my intention. The CSS file updater sounds like a good idea, too, and it might even facilitate the conversion of Trac's CSS files.

[Remy Blank]

#8514 was closed as a duplicate.

[Remy Blank]

I guess we'll have enough incompatible changes in 0.12 without that. Let's do that later.

[Remy Blank]

Refocusing.

[Peter Suter]

Rough mechanical adding of "trac-" to all id's

[Peter Suter]

The above patch is the result of mostly blindly grep & replacing in trac/:

1. Replace id="([^t]|(t[^r])|(tr[^a])|(tra[^c])) by id="trac-\1 but revert changes in:

• any tests
• excanvas.js
• jquery-ui.js and jquery-ui-addons.js
• default-page TracInterfaceCustomization
• resource.py
• formatter.py

2. Repeatedly (until no match found) replace in *.css files #(([^t]|(t[^r])|(tr[^a])|(tra[^c])).*\{) by #trac-\1.

3. Manually fix trac/htdocs/js/wikitoolbar.js.

This would of course first need a very thorough review, testing and maybe reconsideration of exactly which id's need to be prefixed.

[Peter Suter]

One id that was missed by the above was traceback (because it starts with trac). Maybe |(trac[^-]) should be added to the regular expressions above to also catch such cases.

[Christian Boos]

r'id="(?!trac-)([^"]+)"' ;-)

The change can be done for 1.1.1.

We've not discussed yet what's going to happen for 1.1.x, but this will definitely be the place for disruptive changes like this one.

[Peter Suter]

Replying to psuter:

mostly blindly grep & replacing

Unsurprisingly, it's more complicated than that:

• With the (?!trac-) guard, now tracpowered slips through instead of traceback.
• There are some CSS id selectors not followed by a {:
  • 1x #tabs in admin.css.
  • 3x #overview in diff.css.
  • 2x #batchmod_form in report.css.
  • 3x #fieldhist, 4x #content and 2x #systeminfo in trac.css.
• HTML label elements' for attributes contain id's.
  • for=" → for="trac-
• HTML td elements' headersattributes contain id's.
  • headers=" → headers="trac-
• Javascript / jQuery selectors contain id's.
  • (\$\("[^#"]*)#(?!trac-) → \1#trac-
• Functional tests use id's.
  • formvalue\('([^']+)' → formvalue('trac-\1'
  • …
• And more…

[Christian Boos]

I found two issues:

• $("#...") → $"#trac-...")
• in trac/ticket/templates/ticket_preview.html, id="trac-${'trac-change-%d-%d' ...

Also, you didn't modify the classes, is that saved for later or you think we shouldn't do it?

Finally, I think that the (shell/sed/Python) script you used for doing this should be part of the patch (in contrib), so that plugin authors or people having their own extensive CSS customizations could reuse it as well.

[Peter Suter]

Yes, that sounds like a good idea.

[Peter Suter]

attachment:T5997-css-prefix-script.patch​ contains a Python script that does most of the automatable steps mentioned above. attachment:T5997-css-prefix-script-fixups.patch​ contains some fix-ups on top of that. These didn't really seem worth automating.

(The script still contains a commented out rule that used the cssutils Python module. I thought this might be cleaner and more stable. But I couldn't get it to work without also pretty-printing the CSS in a different style. So I replaced it with a custom hack again. For now I only left it in case anyone else has a good idea how to improve it.)

As for plugin and customization authors reusing the script: It might be possible to customize it for such purposes (with manual review and fixing), but getting a 100% automated solution seems unlikely to me. For small plugins and customizations it will probably be simpler to update everything entirely manually.

I still didn't modify any classes, just because getting the id's right first seems tricky enough. Actually, I'm not really sure the benefits are worth the risk. I would be surprised if there weren't still quite some id references I missed.

[Christian Boos]

Great! Looks good, I'm going to integrate it in +testing.

About cssutils: if the pretty-printing it does is looking good (and keeps our comments), we could do a first pass of pretty-printing on our current css, then do the changes as a second step?

[Peter Suter]

(Updated patch to fix @media handling.)

The pretty-printing keeps the comments. It ​can't preserve some CSS hacks though.

[Ryan J Ollos]

Retargetting tickets to narrow focus for milestone:1.2. Please move the ticket back to milestone:next-dev-1.1.x if you intend to resolve it by milestone:1.2.