< IE9 sets width/height attribute once, and doesn't update on other loads #2129

Closed
ibolmo opened this Issue Nov 19, 2011 · 13 comments

Comments

Projects
None yet
3 participants
@ibolmo
Member

ibolmo commented Nov 19, 2011

All non-IE browsers correctly reset an image's width/height after the user has set a new src attribute. I've confirmed with the help of Gonchuki and Sanford from the user mailing list that the soluton is to remove the properties.

Here's a jsfiddle showing the fix.

Element.Properties.src = {
    set: function(src){
       this.src = src;
       return this.removeProperty('width').removeProperty('height');
    } 
};

Here's the fix with boilerplate:

/* <IE> */
if (Browser.ie) Element.Properties.src = {
    set: function(src){
       this.src = src;
       return this.removeProperty('width').removeProperty('height');
    } 
};
/* </IE> */

I need other devs to review the fix and I can create a PR.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 19, 2011

Umm srsly a UA sniff?

jdalton commented Nov 19, 2011

Umm srsly a UA sniff?

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 19, 2011

Would feel better if the width/height attributes were removed before the src was set (I know it may be async but it just feels right)

jdalton commented Nov 19, 2011

Would feel better if the width/height attributes were removed before the src was set (I know it may be async but it just feels right)

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 19, 2011

Also I think a quick review of elements w/ src attributes would be good to see if it makes sense to remove width/height on all of them.

jdalton commented Nov 19, 2011

Also I think a quick review of elements w/ src attributes would be good to see if it makes sense to remove width/height on all of them.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 19, 2011

I remember running into issues similar to this in 2005/2006 and having to come up with a way to find an images natural height/width... will dig a bit later to see what I can come up with.

jdalton commented Nov 19, 2011

I remember running into issues similar to this in 2005/2006 and having to come up with a way to find an images natural height/width... will dig a bit later to see what I can come up with.

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 19, 2011

Member

Yeah the UA sniff was a quick solution, I didn't invest on thinking on the best feature detection.

I'm not against removing the width and height prior to setting the src.

Good call on checking against img tag. Not going to be a problem with 2.0, hehe.

I'll wait for your 2k5/2k6 research, and create the PR afterwards.

Member

ibolmo commented Nov 19, 2011

Yeah the UA sniff was a quick solution, I didn't invest on thinking on the best feature detection.

I'm not against removing the width and height prior to setting the src.

Good call on checking against img tag. Not going to be a problem with 2.0, hehe.

I'll wait for your 2k5/2k6 research, and create the PR afterwards.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 20, 2011

Ok so I did some research and it seems the reason IE isn't changing the image size is that it is automatically assigning it a height/width attribute. The other browsers resize the image because there is no explicitly set dimensions.

Because IE auto adds these attributes it may be tricky to detect if they were explicitly assigned or auto so simply clearing them is not a good idea because they may be added on purpose.

(Maybe adding a dev note somewhere would help)

jdalton commented Nov 20, 2011

Ok so I did some research and it seems the reason IE isn't changing the image size is that it is automatically assigning it a height/width attribute. The other browsers resize the image because there is no explicitly set dimensions.

Because IE auto adds these attributes it may be tricky to detect if they were explicitly assigned or auto so simply clearing them is not a good idea because they may be added on purpose.

(Maybe adding a dev note somewhere would help)

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 21, 2011

Member

Hrm.. I couldn't find what the spec mentions about width/height attribute
vs CSS with the definition of width and height, but perhaps we can take
their decision to drop cellpadding cellspacing in favor of margin and
padding in table.

And since they're using Element.set('src') rather than just element.src,
perhaps with documentation it might suffice to explain the convention on IE
only.

On Sun, Nov 20, 2011 at 3:30 PM, John-David Dalton <
reply@reply.github.com

wrote:

Ok so I did some research and it seems the reason IE isn't changing the
image size is that it is automatically assigning it a height/width
attribute. The other browsers resize the image because there is no
explicitly set dimensions.

Because IE auto adds these attributes it may be tricky to detect if they
were explicitly assigned or auto so simply clearing them is not a good idea
because they may be added on purpose.


Reply to this email directly or view it on GitHub:
#2129 (comment)

Member

ibolmo commented Nov 21, 2011

Hrm.. I couldn't find what the spec mentions about width/height attribute
vs CSS with the definition of width and height, but perhaps we can take
their decision to drop cellpadding cellspacing in favor of margin and
padding in table.

And since they're using Element.set('src') rather than just element.src,
perhaps with documentation it might suffice to explain the convention on IE
only.

On Sun, Nov 20, 2011 at 3:30 PM, John-David Dalton <
reply@reply.github.com

wrote:

Ok so I did some research and it seems the reason IE isn't changing the
image size is that it is automatically assigning it a height/width
attribute. The other browsers resize the image because there is no
explicitly set dimensions.

Because IE auto adds these attributes it may be tricky to detect if they
were explicitly assigned or auto so simply clearing them is not a good idea
because they may be added on purpose.


Reply to this email directly or view it on GitHub:
#2129 (comment)

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 23, 2011

Member

@jdalton, @cpojer, and @arian. What'd you think of my last comment.

Member

ibolmo commented Nov 23, 2011

@jdalton, @cpojer, and @arian. What'd you think of my last comment.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Nov 24, 2011

+1 for doc note

jdalton commented Nov 24, 2011

+1 for doc note

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Dec 10, 2011

Member

doc note is fine.

Member

arian commented Dec 10, 2011

doc note is fine.

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Dec 11, 2011

Member

Ok I may have an actual boss solution. Since it's so late, I'll document it and either one of you guys get around to it before I do (or refute it) just follow-up here.

Take a look at: http://jsfiddle.net/XS6Kw/

DOM 2 (or whatever) added a specified attribute to an Attr node. Since IE6 it's been supported. The idea is that there is a way to know if an attribute had been previously been specified by the user. Either by html or programmatically (el.width =). This is by using the specified attribute in the attribute node.

This assumes that Element.set('src') is being used, otherwise burden falls to the developer. Element.set('src', ...) remove attribute nodes width and height (not necessarily mutually) if they haven't been specified. If specified, the question falls to what the other browsers do (TODO). If the user specifies a width attribute and changes the src to a different sized element, I assume that the browser will honor the specified attribute and not look at the new image sizes. Therefore, in IE we'd just let those attributes stick.

That's it for now. It's a good move forward, imo.

Member

ibolmo commented Dec 11, 2011

Ok I may have an actual boss solution. Since it's so late, I'll document it and either one of you guys get around to it before I do (or refute it) just follow-up here.

Take a look at: http://jsfiddle.net/XS6Kw/

DOM 2 (or whatever) added a specified attribute to an Attr node. Since IE6 it's been supported. The idea is that there is a way to know if an attribute had been previously been specified by the user. Either by html or programmatically (el.width =). This is by using the specified attribute in the attribute node.

This assumes that Element.set('src') is being used, otherwise burden falls to the developer. Element.set('src', ...) remove attribute nodes width and height (not necessarily mutually) if they haven't been specified. If specified, the question falls to what the other browsers do (TODO). If the user specifies a width attribute and changes the src to a different sized element, I assume that the browser will honor the specified attribute and not look at the new image sizes. Therefore, in IE we'd just let those attributes stick.

That's it for now. It's a good move forward, imo.

@ibolmo ibolmo referenced this issue Dec 12, 2011

Closed

Fixes 2129. #2168

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Feb 5, 2012

Member

I vote for a doc note. I have to say that I didn't really tested this, but I have this feeling this might open the door for even more bugs and edge cases. As I understand with a documentation note it's not too hard to work around this, as a user.

Member

arian commented Feb 5, 2012

I vote for a doc note. I have to say that I didn't really tested this, but I have this feeling this might open the door for even more bugs and edge cases. As I understand with a documentation note it's not too hard to work around this, as a user.

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Feb 6, 2012

Member

Going for the doc.

Member

ibolmo commented Feb 6, 2012

Going for the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment