Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues after upgrading #120

Closed
23doors opened this issue Sep 14, 2011 · 18 comments
Closed

Issues after upgrading #120

23doors opened this issue Sep 14, 2011 · 18 comments

Comments

@23doors
Copy link

23doors commented Sep 14, 2011

After upgrading django_compressor==0.9.2 to newest django_compressor==1.0.1 via pip I am experiencing issues with css compression.

{% compress css %}
    <link type="text/css" rel="stylesheet" href="/static/www/css/reset.css" media="screen"/>
    <link type="text/css" rel="stylesheet" href="/static/www/css/text.css" media="screen"/>
    <link type="text/css" rel="stylesheet" href="/static/www/css/start/main.css" media="screen"/>
    <link type="text/css" rel="stylesheet" href="/static/www/css/ui.selectmenu.css" media="screen"/>
{% endcompress %}

This block previously worked normally, right now with both DEBUG True and DEBUG False only first css is displayed.
I can remove first css (reset) from the block and then only second one (text) is used.
I found that when I disable COMPRESS_PRECOMPILERS it all works.

My COMPRESS_PRECOMPILERS is pretty straightforward and works with previous version:
COMPRESS_PRECOMPILERS = (
('text/coffeescript', 'coffee --compile --stdio'),
('text/less', 'lessc {infile} {outfile}'),
)

@gabrielgrant
Copy link

I think I may have just ran into the same (or a related) problem. For me, it seems the problem stems from the name change of the COMPRESS setting to COMPRESS_ENABLED.

Changing COMPRESS to COMPRESS_ENABLED in my settings.py fixed things.

Out of curiosity, why was the name of this setting changed?

@23doors
Copy link
Author

23doors commented Sep 14, 2011

Unfortunately this does not change anything for me. Setting COMPRESS_ENABLED/COMPRESS or removing it from settings doesn't change a thing. Only removing (commenting) COMPRESS_PRECOMPILERS works.

@gerardo
Copy link

gerardo commented Sep 15, 2011

I can confirm this. I just migrated from django-mediagenerator.

As @23doors points out, commenting COMPRESS_PRECOMPILERS is needed to show all css links. If COMPRESS_PRECOMPILERS is present, it just shows the first defined css in the block.

@gerardo
Copy link

gerardo commented Sep 15, 2011

Here's a patch: https://gist.github.com/1218346

CompressorNode.debug_mode just checks for COMPRESS_DEBUG_TOGGLE, and my guess is that it should first check for settings.DEBUG, based on the method name :)

I'm not sure if that's what you intended to do with debug_mode.

@redneckbeard
Copy link

@23doors, we had the exact same thing happen.

The issue results from a great new feature in 1.0 -- precompilers being applied to {% compress %} blocks when compression is disabled. What wasn't taken into account was that CSS assets get bundled by the media attribute on the link/style tags, and skipping the actual compression phase means that anything after the initial tag for a given media type disappears into the abyss.

Fixed in our fork here: https://github.com/threepress/django_compressor/commit/f94c05e8930d793a9383d8f0170e50960fcc4570

@jezdez
Copy link
Member

jezdez commented Sep 15, 2011

I think this is related to #119, which I just pushed a fix for, please try it out.

@redneckbeard
Copy link

That seems to fix it for me.

@redneckbeard
Copy link

I take that back -- I still had COMPRESS_PRECOMPILERS commented out. The patch for #119 does not fix this for me; my patch linked above does.

I'll try to get a regression test written this evening.

@jezdez
Copy link
Member

jezdez commented Sep 15, 2011

Okay, thanks, that'd be appreciated.

@23doors 23doors closed this as completed Sep 19, 2011
@23doors 23doors reopened this Sep 19, 2011
@jezdez
Copy link
Member

jezdez commented Sep 21, 2011

Any progress on tests?

@redneckbeard
Copy link

Sorry, have been quite sidetracked. Hoping to push something this weekend.

@ghost
Copy link

ghost commented Sep 25, 2011

This looks like it's related to the issue I posted before about the CSS. #103

@nigma
Copy link

nigma commented Oct 13, 2011

I can confirm that the patch https://github.com/threepress/django_compressor/commit/f94c05e8930d793a9383d8f0170e50960fcc4570 fixes the issue of missing css links for COMPRESS_ENABLED=False and non-empty COMPRESS_PRECOMPILERS.

Precompilers are still executed when necessary and the rest of css links goes through as expected.

Also please note that this issue is specific only to CssCompressor. JsCompressor is unaffected.

@gerardo
Copy link

gerardo commented Oct 13, 2011

Did you test it with DEBUG = True?

@jezdez
Copy link
Member

jezdez commented Nov 2, 2011

I'm sorry but I can't reproduce this, closing as long there isn't an actual test case proofing this issue.

@jezdez jezdez closed this as completed Nov 2, 2011
@23doors
Copy link
Author

23doors commented Dec 6, 2011

@jezdez
Take a look at: https://github.com/23doors/compressor_issue
I created very raw basic project (direct to template, no settings changed from default django startapp except MEDIA_ROOT, COMPRESS_CSS_FILTERS, COMPRESS_PRECOMPILERS).
Not a test case but should do.
When you runserver and go to main page - you get only 1 css in source (there are two).

This issue occurs when DEBUG = True (so COMPRESS_ENABLED=False) and COMPRESS_PRECOMPILERS are not empty (I guess it does some processing that should only be done when COMPRESS_ENABLED is True).

https://github.com/threepress/django_compressor/commit/f94c05e8930d793a9383d8f0170e50960fcc4570 fixes this.
But I haven't tested with css files that actually needs precompiling (less etc).

@jezdez jezdez reopened this Dec 8, 2011
@23doors
Copy link
Author

23doors commented Dec 9, 2011

Scratch that. I did some tests with memcached and s3. Eventually memcached was storing some wrong cache values for me. Commit above seems to be working.

@peterlundberg
Copy link
Contributor

I can confirm that the issue described by 23doors is still exists in 1.1. I have created a pullrequest #194 that fixes this (tested in a mixed css/less project) based on thresspress original fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants