I think it would be better if the CSS rules were stored authoritatively in the database. Otherwise, they can be lost when moving a site (which would expect this to be in the db).

Whenever a rule is saved, it should be written out to the files directory.

There should probably be a "refresh all files" option that would write out all rules. This could be used when a database was moved.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klonos’s picture

I can see how this (storing rules in the db) would be useful to some. Thinking ahead and suggesting that the rules should at the same time be exported in actual files was a great catch, since some people like to edit their css in raw files (through their ftp client for example). What happens when this procedure happens in reverse though? One edits their files in raw. Does the db get updated then with the new data? How should this update be triggered?

klonos’s picture

There should probably be a "refresh all files" option that would write out all rules. This could be used when a database was moved.

...this would also -partially- solve this perhaps(?): #886188: Export/Features capability for CSS Injector

-edit: stupid me + late night, forget to close my tags properly-

rfay’s picture

Since this issue and #886188: Export/Features capability for CSS Injector were created within one minute of each other, I'm betting they're related thoughts :-)

The database would become the authoritative source of CSS rules, and anybody who edited the files would lose them. IMO that's OK. If we wanted to add user-provided files as in one of the other issues, that would be a different matter.

klonos’s picture

The database would become the authoritative source of CSS rules, and anybody who edited the files would lose them. IMO that's OK.

I personally prefer the file storage, because I find it convenient to edit the files from the command line or through my ftp client's UI. If we implement db storage as the sole way, then files will only serve as an automated kind of backup/export (partially fixing #886188: Export/Features capability for CSS Injector). The way it currently works is that one is able to edit rules either from the module's UI or 'manually' and both will reflect each other's changes. I thing we need to figure out a way to make this work both ways with the new db storage system too, or else this is loss of functionality and that is not OK at all (the way I see it). It is functionality useful to developers/themers too, so not something that could easily be overlooked. I am curious to see what others think on this one.

PS: I am sure there'll be flame wars over this one. If it is to change anyways, I'd suggest a 6.x-2.x/7.x-2.x branch for the new storing mechanism. Either that, or alternatively we can offer both options through the use of submodules perhaps(?), I don't know.

nicksanta’s picture

If you're already using FTP to edit the generated files directly, I ask you the following:
1. Why do you need a GUI module to manage it?
2. Why are you editing a css file in sites/default/files which can be overwritten and/or deleted by the webserver, essentially making it a dangerous place to keep mision-critical styles?

I *really* don't think you need to bloat out a module like this to support edge-case use cases.

IMHO, db storage is the way to go, with an imagecache-esque "flush" command.

klonos’s picture

First off, I need those files to be generated first, and I can do this only through the UI.

1. I then need the UI in order to be able to set included/excluded pages.

2a. Databases can be dropped/corrupted as easily as files can be deleted.
2b. The css files used simply to do some alteration in themes are far from mission-critical (I use css injector to add my code and not have to manually re-apply changes each time a theme is updated to a newer version - I don't use it to theme from scratch).
2c. Even if the .css files get deleted, who says one cannot have backups of them?

So Nick, you actually believe that the more than 6.000 sites using this module the way it is now are 'edge-case use cases', huh? I guess then that the way one uses the module actually determines how they feel about this feature discussed here. For me -for example- the .css code editor part of the rule edit form is of little use -if not any at all- once the file is crated. I realize though that it is a really useful feature for those that do not manually touch those files, so I am not suggesting that it should be removed simply because I don't use it.

I guess what I am trying to say is that I am not 'voting against' this feature here, so long as it doesn't remove old functionality. I am trying to figure out a 'gold line' that would satisfy both sides.

PS: I was right back in #4 when I predicted that this would end in flames ;)

nicksanta’s picture

Whoah, easy tiger. No flaming intended.

My comment regarding "edge-case" use cases was referring to modifying files generated by css injector using an FTP client.

A suggestion, maybe a custom subtheme would be a better idea?

Regardless, there is no reason why you couldn't continue working as normal if #886188: Export/Features capability for CSS Injector came through. With the small exception of editing a file in the feature module instead of a random file in sites/default/files

zkrebs’s picture

Just a heads up, I really love this module and its working great.

That being said,

An import/export feature would be wonderful.

geek-merlin’s picture

+1 for #7 - move forces to export / features

bailey86’s picture

The .css could be stored in the database and the .css files created on the fly as per imagecache. This might not be wanted by people who directly edit the .css files themselves - but I see this as something which should have been clear from the beginning.

If people want to edit the css files manually then maybe there should be an export/import css file feature to allow for the css to be exported to a file which can then be edited manually and then imported back in to the database.

bibo’s picture

I had been in the understanding that css_injector already did both storage types (file and database as backup)... but I now noticed this isn't the case.

I would love to see the CSS stored in both (or even database only), and/or the import/export feature. With it I would recommend and use this module on almost any site, but without it I couldn't fully rely on this module. A deleted file could mean a catastrophe, especially the /files/ - folder is usually kept out of version control.

Does anyone happen to be working on this feature?

klonos’s picture

Version: 6.x-1.4 » 7.x-1.x-dev

I had been in the understanding that css_injector already did both storage types...

Nope! ...this issue would have been marked "fixed" if this was the case.

This needs to go against latest dev first, then backported to 6.x-dev and hit the stable of both branches.

xtfer’s picture

+1 for seeing this, especially in 6.x.

Not having the CSS stored somewhere it can be properly transferred between development and production environments is a real issue, And, I agree with @nicksanta in #5. Files directories shouldn't be version controlled, and are a poor place to store configuration.

xtfer’s picture

Version: 7.x-1.x-dev » 6.x-1.4
Status: Active » Needs review
FileSize
5.44 KB

This patch add database storage for CSS Injector CSS.

Its written against 6.x, as thats what I needed it for, but I suspect it would be relatively easy to port it to 7.x.

Functionally, the module still works the same way, it just saves CSS to the database as well as the file store. It ONLY does this when a rule is saved, currently there is no regeneration on cache clear (there probably should be).

There is also a checkbox on each rule to load that CSS from the file system instead of the database, to satisfy @klonos' requirement.

Status: Needs review » Needs work

The last submitted patch, 886184-store-css-in-database.patch, failed testing.

xtfer’s picture

Version: 6.x-1.4 » 6.x-1.x-dev
Status: Needs work » Needs review
FileSize
5.03 KB

Sorry, patched against the wrong codebase...

mototribe’s picture

a patch for D7 would be great.
I can't push the css_injector files because they files directory is excluded from git access on my host (Pantheon).

MiSc’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev

For D7, added support for ctools exportables: http://drupal.org/node/886188#comment-7304230

podarok’s picture

Issue summary: View changes
Parent issue: » #2158951: [META] CSS Injector 7.x-2.x roadmap
podarok’s picture

My 5 cents
This feature looks like not needed if we convert css_injector rules to revisionable entities #2046855: Add feature to keep revisions for a rule

podarok’s picture

Assigned: Unassigned » podarok
Status: Needs review » Postponed
kreynen’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Postponed » Needs review

I've restarted the 2.x work on CSS Injectors based on the improvement already made to JS Injector. Still working on the update hook to from 1.x -> 2.x, but CSS for each "rule" is now stored in the database, is exportable/importable, and plays well with Features.

We had to move clients hosted on Pantheon to the 2.x branch due to a bug in Pantheons backup process so I feel this is really getting a fair amount of use, but I'd like more feedback from people more familiar with the 1.x version.