No support for versions
Crell - January 14, 2009 - 07:16
| Project: | CSS |
| Version: | 6.x-1.0-rc1 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | fax8 |
| Status: | needs review |
Description
The current implementation does not support node versions at all. It needs to, as otherwise there is no way to roll back CSS changes along with a revision.
#1
I agree. I'm working on this one.
#2
The attached patch implements support for node revisions in css module.
Please test this patch. These are the tests I'm interested:
#3
The above patch change the behaviour of using @import "?q=css/get/x"; to reference the CSS rules of other nodes.
Before x was the nid (node id) while it is now the vid (version id).
I'm not sure if it could be useful also a way to reference the latest CSS version of a css attached to a node.
With the implementation in the patch if one reference a CSS attached to a specified version of a node, and then a new version of the referenced css is available, nodes referencing the old version will be not updated with the new versions.
Probably the best workaround would be provide another hook (eg. css/get_nid/x) where x is now the nid. This hook would always returns the latest revision of the CSS.
Any other idea?
#4
I'd actually argue that we should move away from the dynamic call, as that results in two hits against Drupal. Instead, do what imagecache does a and feed in a path to drupal_add_css() (that may need to be uncacheable) that points to $files_dir/nodecss/$nid, which in turn is a menu callback that generates a file at that location. That would avoid the second hit against Drupal itself on subsequent requests. That's probably a separate issue, though.
It probably does make sense to go by vid here, though. I'll try to test this patch when sometime soon.
#5
Hi,
Just chasing this up.. Has this patch been tested yet and if so what feedback if any is there?
- Whispero
#6
Unfortunately I had a number of things explode on me in the past few weeks so I've not been able to do so. I'll let you know when I do. :-/
#7
Ok Crell,
No probs
- Whispero
#8
Still looking for feedback for the patch above. Anyone helps?