Side navigation
#13088 closed bug (fixed)
Opened December 19, 2012 08:30AM UTC
Closed December 31, 2012 05:29PM UTC
under IE8, $(selector).attr('style') always return lowercase string
Reported by: | superbible@gmail.com | Owned by: | mikesherov |
---|---|---|---|
Priority: | low | Milestone: | 1.9 |
Component: | css | Version: | 1.8.3 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
On IE8, due to $(selector).attr('style') always return lowercase string, this will lead background image missing.
I checked the source code, it's below:
if ( !jQuery.support.style ) {
jQuery.attrHooks.style = {
get: function( elem ) {
Return undefined in the case of empty string
Normalize to lowercase since IE uppercases css property names
return elem.style.cssText.toLowerCase() || undefined;
},
set: function( elem, value ) {
return ( elem.style.cssText = value + "" );
}
};
}
see this part:
// Normalize to lowercase since IE uppercases css property names
return elem.style.cssText.toLowerCase() || undefined;
This works in most cases except when style contains URL portions, for example: background-image:url('xxx.com/Path/MyImage.png')
the URL is case-sensitive, so above process will return lowercase string lead to the image can't be loaded.
Attachments (0)
Change History (10)
Changed December 19, 2012 08:38AM UTC by comment:1
Changed December 21, 2012 04:11AM UTC by comment:2
owner: | → superbible@gmail.com |
---|---|
status: | new → pending |
This seems like a choice between two horrible alternatives, one of which people have come to expect. Most code is going to really dislike uppercase style names. I understand that lowercasing file names can corrupt them.
Why look at the style information this way? Can't you just get it through .css()
if you need the URL with the original case?
Changed December 21, 2012 12:04PM UTC by comment:3
_comment0: | Replying to [comment:2 dmethvin]: \ > This seems like a choice between two horrible alternatives, one of which people have come to expect. Most code is going to really dislike uppercase style names. I understand that lowercasing file names can corrupt them. \ > \ > Why look at the style information this way? Can't you just get it through `.css()` if you need the URL with the original case? \ \ We build a WYSIWIG theme designer let user to customize their own theme, so user dynamically set css propery to elements, then we need to save them, so it's very simple to get what user change and need to save by $(element).attr('style'). \ If use prop('style'), it'll get more things than I want. \ Now I just add a patch to JQuery by disabled the code mentioned in the bug report. \ → 1356091549812618 |
---|
Replying to [comment:2 dmethvin]:
This seems like a choice between two horrible alternatives, one of which people have come to expect. Most code is going to really dislike uppercase style names. I understand that lowercasing file names can corrupt them. Why look at the style information this way? Can't you just get it through .css()
if you need the URL with the original case?
We build a WYSIWIG theme designer let user to customize their own theme, so user dynamically set css propery to elements, then we need to save them, so it's very simple to get what user change and need to save by $(element).attr('style').
So to use .css() is not suitable.
If use prop('style'), it'll get more things than I want.
Now I just add a patch to JQuery by disabled the code mentioned in the bug report.
Changed December 21, 2012 12:11PM UTC by comment:4
Thanks for contributing!
If we really wanted to fix this, it requires splitting cssText, iterating over each declaration, fixing just the property name by actually camel casing it properly (as opposed to just lower casing it, which is impossible without a map of proper casings), and then joining together.
Instead, as dmethvin points out, just use .css(). Or if you need the specified inline cssText, use [0].style.cssText (native). In fact, we should probably remove this attrHook altogether, as its pretty archaic at this point and in fact just wrong.
Thoughts on removing it, anyone?
Changed December 21, 2012 03:32PM UTC by comment:5
Removing it saves us some bytes, although I can see that breaking code (suddenly, UPPERCASE). I suppose though that we are clearly destroying information by lowercasing the text. If someone wants to lowercase it themselves they can still do that. So my vote would be to remove this.
Changed December 21, 2012 04:03PM UTC by comment:6
To be clear, I meant to keep the hook, but stop the lowercasing.
Changed December 25, 2012 07:25AM UTC by comment:7
Replying to [comment:4 mikesherov]:
Thanks for contributing! If we really wanted to fix this, it requires splitting cssText, iterating over each declaration, fixing just the property name by actually camel casing it properly (as opposed to just lower casing it, which is impossible without a map of proper casings), and then joining together. Instead, as dmethvin points out, just use .css(). Or if you need the specified inline cssText, use [0].style.cssText (native). In fact, we should probably remove this attrHook altogether, as its pretty archaic at this point and in fact just wrong. Thoughts on removing it, anyone?
Thank you for telling me [0].style.cssText.
I think it's OK to remove it if it's not easy to make it work correctly.
Changed December 28, 2012 04:52PM UTC by comment:8
component: | unfiled → css |
---|---|
milestone: | None → 1.9 |
priority: | undecided → low |
status: | pending → open |
Changed December 31, 2012 03:45PM UTC by comment:9
owner: | superbible@gmail.com → mikesherov |
---|---|
status: | open → assigned |
Changed December 31, 2012 05:29PM UTC by comment:10
resolution: | → fixed |
---|---|
status: | assigned → closed |
Fixes #13088: under IE8, $(selector).attr('style') always return lowercase string
Changeset: abead1c86b75d376544e52cd916031a67caf2c34
I think we should remove this code.
And I didn't find it in JQuery1.4.4, which works for me.