[#SMARTSPRITES-23] Support of !important in background-image attribute - Carrot2 Clustering Engine Issues

Details

Type: Bug
Status: Open
Priority: Critical
Resolution: Unresolved
Affects Version/s: 0.2.0
Fix Version/s: 0.4.0
Component/s: None

Description:

Hide

I have a lot of background-image definitions which have the !important keyword added in the end like this:

background-image: url(../js/grid/page-first.gif) !important;

But currently it's not possible to annotate this with SmartSprite like this:

background-image: url(../js/grid/page-first.gif) !important; /** sprite-ref: hsprite; */

I think this is a vital point - unless there's a workaround for this case.

Show
I have a lot of background-image definitions which have the !important keyword added in the end like this: background-image: url(../js/grid/page-first.gif) !important; But currently it's not possible to annotate this with SmartSprite like this: background-image: url(../js/grid/page-first.gif) !important; /** sprite-ref: hsprite; */ I think this is a vital point - unless there's a workaround for this case.

Issue Links

Dependency

This issue depends on:
SMARTSPRITES-47 Replace line-based CSS processing with a parser-based one

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Stanisław Osiński added a comment - 14/Oct/08 06:39 PM

Good point, Philipp! I can't think of an immediate workaround, but I'll try to fix this ASAP.

Show

Stanisław Osiński added a comment - 14/Oct/08 06:39 PM Good point, Philipp! I can't think of an immediate workaround, but I'll try to fix this ASAP.

Hide

Permalink

Stanisław Osiński added a comment - 14/Oct/08 09:55 PM

Hi again,

I've put together a quick fix, you can download the updated version from:

http://smartsprites.osinski.name/download/smartsprites-0.2.1-alpha.zip

(or you can use the latest trunk from svn)

One thing that we cannot solve nicely (until I implement SMARTSPRITES-22 you filed) is this: if the original background-image property has an "!important" modifier (just like in your example) – should the generated background-position property also have the "!important" modifier? For now, I assumed that both the generated background-image and background-position would have it, but I'm not sure how that matches your specific case.

Anyhow, I'm curious about your feedback.

Show

Stanisław Osiński added a comment - 14/Oct/08 09:55 PM Hi again, I've put together a quick fix, you can download the updated version from: http://smartsprites.osinski.name/download/smartsprites-0.2.1-alpha.zip (or you can use the latest trunk from svn) One thing that we cannot solve nicely (until I implement SMARTSPRITES-22 you filed) is this: if the original background-image property has an "!important" modifier (just like in your example) – should the generated background-position property also have the "!important" modifier? For now, I assumed that both the generated background-image and background-position would have it, but I'm not sure how that matches your specific case. Anyhow, I'm curious about your feedback.

Hide

Permalink

Philipp Rosenhagen added a comment - 14/Oct/08 11:27 PM

Nice! I'll try this tomorrow.

As you already said: until #22 is not implemented there is some gray area on how to manage the !important keyword for the attributes to set.
At this stage I'm just replacing the existing ones which have "background-image" attributes - and not the ones with the short syntax "background". But even when the "background:" attribute have the !important flag it should be preserved (that's relevant for #22).
For the current case - yes - your assumption should be right. Because the !important keyword overwrites effectively other definitions (added via css inheritance) - the background-image have to be in the right place - and that's only savely possible if you apply the !important keyword to the background-position attribute as well. Maybe it gets more complicated when there is also a "background-position" attribute in the original css with the keyword !important... which takes precedence? The one defined by the user or the one generated by SmartSprites? In the FAQ you advice that those attributes should be positioned before the SmartSprites-annotated background-image attribute. So I think currently the generated one overwrites the original position. I think that's ok.

Mmm... what about this case:

div.foo {
background-image: url(bar.jpg) !important; /** sprite-ref: hsprite; */
}
div {
background-position: -10px 20px;
}

The generated css by SmartSprites would be:

div.foo {
background-position: 0px 10px !important; /* example values - depending on the image dimensions */
background-image: url(hsprite.png) !important;
}
div {
background-position: -10px 20px;
}

The second definition would be overwritten by the !important keyword added by SmartSprites. So maybe this interferes the layout intended by the designer. So I think the FAQ should be extended by this issue. Overwriting styles via inheritance can not be detected by SmartSprites (because the reformating can occur in every style after and outside the currently processing style definition). And this is not just a problem when using the !important keyword. So when using SmartSprites - this fact should always be kept in mind.

Show

Philipp Rosenhagen added a comment - 14/Oct/08 11:27 PM Nice! I'll try this tomorrow. As you already said: until #22 is not implemented there is some gray area on how to manage the !important keyword for the attributes to set. At this stage I'm just replacing the existing ones which have "background-image" attributes - and not the ones with the short syntax "background". But even when the "background:" attribute have the !important flag it should be preserved (that's relevant for #22). For the current case - yes - your assumption should be right. Because the !important keyword overwrites effectively other definitions (added via css inheritance) - the background-image have to be in the right place - and that's only savely possible if you apply the !important keyword to the background-position attribute as well. Maybe it gets more complicated when there is also a "background-position" attribute in the original css with the keyword !important... which takes precedence? The one defined by the user or the one generated by SmartSprites? In the FAQ you advice that those attributes should be positioned before the SmartSprites-annotated background-image attribute. So I think currently the generated one overwrites the original position. I think that's ok. Mmm... what about this case: div.foo { background-image: url(bar.jpg) !important; /** sprite-ref: hsprite; */ } div { background-position: -10px 20px; } The generated css by SmartSprites would be: div.foo { background-position: 0px 10px !important; /* example values - depending on the image dimensions */ background-image: url(hsprite.png) !important; } div { background-position: -10px 20px; } The second definition would be overwritten by the !important keyword added by SmartSprites. So maybe this interferes the layout intended by the designer. So I think the FAQ should be extended by this issue. Overwriting styles via inheritance can not be detected by SmartSprites (because the reformating can occur in every style after and outside the currently processing style definition). And this is not just a problem when using the !important keyword. So when using SmartSprites - this fact should always be kept in mind.

Hide

Permalink

Philipp Rosenhagen added a comment - 27/Oct/08 01:08 PM

Sorry for the late reply.

The Unable to render embedded object: File (important thing works as desired - thanks) not found.

Show

Philipp Rosenhagen added a comment - 27/Oct/08 01:08 PM Sorry for the late reply. The Unable to render embedded object: File (important thing works as desired - thanks) not found.

Hide

Permalink

Stanisław Osiński added a comment - 27/Oct/08 07:05 PM

Is the "The Unable to render embedded object: File" some kind of error you are getting?

Show

Stanisław Osiński added a comment - 27/Oct/08 07:05 PM Is the "The Unable to render embedded object: File" some kind of error you are getting?

Hide

Permalink

Philipp Rosenhagen added a comment - 27/Oct/08 07:55 PM

lol - no - this looks like a bug in the JIRA software .. I'm pretty sure I've just posted "The !important thing works as desired - thanks" ... it looks like that the parser of the comment-function is trying to transform this to something else

Show

Philipp Rosenhagen added a comment - 27/Oct/08 07:55 PM lol - no - this looks like a bug in the JIRA software .. I'm pretty sure I've just posted "The !important thing works as desired - thanks" ... it looks like that the parser of the comment-function is trying to transform this to something else

Hide

Permalink

Stanisław Osiński added a comment - 27/Oct/08 07:58 PM

Interesting Thanks for the report!

Show

Stanisław Osiński added a comment - 27/Oct/08 07:58 PM Interesting

Thanks for the report!

Hide

Permalink

Stanisław Osiński added a comment - 25/Jul/09 11:17 AM

Moving out of the 0.3.0 release.

Show

Stanisław Osiński added a comment - 25/Jul/09 11:17 AM Moving out of the 0.3.0 release.

People

Assignee:

Stanisław Osiński

Reporter:

Philipp Rosenhagen
Votes:

0

Watchers:

0

Dates

Created:

14/Oct/08 05:28 PM

Updated:

26/Jul/09 10:05 PM

Time Tracking

Estimated:

Not Specified

Remaining:

Logged:

50m