IE7/8 getProperty returns functions #2109

Closed
bastianschilbe opened this Issue Oct 19, 2011 · 8 comments

Comments

Projects
None yet
4 participants
@bastianschilbe

$('element').getProperty('inject')

FF/Safari/IE9 return
5

IE7/8 return
function(el, where){
inserters[where || 'bottom'](this, document.id%28el, true%29);
return this;
}

Element methods seem to be returned.

@ghost ghost assigned arian Oct 19, 2011

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 29, 2011

Member

Yeah.. this is also for IE6. Here's a fiddle: http://jsfiddle.net/tQ4QN/

Member

ibolmo commented Nov 29, 2011

Yeah.. this is also for IE6. Here's a fiddle: http://jsfiddle.net/tQ4QN/

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 29, 2011

Member

Here we go: http://jsfiddle.net/tQ4QN/4/

@fabiomcosta @subtleGradient, this is a tough one ... should Slick.getAttribute fix this?

Member

ibolmo commented Nov 29, 2011

Here we go: http://jsfiddle.net/tQ4QN/4/

@fabiomcosta @subtleGradient, this is a tough one ... should Slick.getAttribute fix this?

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Nov 29, 2011

Member

Actually, this might be mootools related. @arian @cpojer help :D

Member

ibolmo commented Nov 29, 2011

Actually, this might be mootools related. @arian @cpojer help :D

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Dec 3, 2011

Member

Still waiting on input from @fabiomcosta, @subtleGradient, @arian, @cpojer. Looking for opinions/direction or 2c. Thanks!

Member

ibolmo commented Dec 3, 2011

Still waiting on input from @fabiomcosta, @subtleGradient, @arian, @cpojer. Looking for opinions/direction or 2c. Thanks!

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Dec 10, 2011

Member

I don't know, it's unwanted behaviour to me. If it's indeed Slick.getAttribute then it should be fixed there, I wouldn't know how though..

Member

arian commented Dec 10, 2011

I don't know, it's unwanted behaviour to me. If it's indeed Slick.getAttribute then it should be fixed there, I wouldn't know how though..

@fabiomcosta

This comment has been minimized.

Show comment
Hide comment
@fabiomcosta

fabiomcosta Dec 10, 2011

Member

It's not Slick... It's how the elements are extended on IE by Mootools and the way that expandos are threated as attributes on IE. I don't agree we should fix it there since Slick is intended to be used anywhere, by any framework, and should not have anything related to mootools-core inside it.

Agreed?

Member

fabiomcosta commented Dec 10, 2011

It's not Slick... It's how the elements are extended on IE by Mootools and the way that expandos are threated as attributes on IE. I don't agree we should fix it there since Slick is intended to be used anywhere, by any framework, and should not have anything related to mootools-core inside it.

Agreed?

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Dec 10, 2011

Member

But shouldn't it work with other frameworks that do similar things with
element methods in IE?
On Dec 10, 2011 5:30 PM, "Fbio M. Costa" <
reply@reply.github.com>
wrote:

It's not Slick... It's how the elements are extended on IE by Mootools and
the way that expandos are threated as attributes on IE. I don't agree we
should fix it there since Slick is intended to be used anywhere, by any
framework, and should not have anything related to mootools-core inside it.

Agreed?


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

Member

arian commented Dec 10, 2011

But shouldn't it work with other frameworks that do similar things with
element methods in IE?
On Dec 10, 2011 5:30 PM, "Fbio M. Costa" <
reply@reply.github.com>
wrote:

It's not Slick... It's how the elements are extended on IE by Mootools and
the way that expandos are threated as attributes on IE. I don't agree we
should fix it there since Slick is intended to be used anywhere, by any
framework, and should not have anything related to mootools-core inside it.

Agreed?


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

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Dec 11, 2011

Member

The problem is that

el.anything will be considered an attribute by IE. This means that el.getAttribute('anything') will give the value of it.

One solution is to whitelist using setProperty any attributes and check against the property in getProperty if someone wants el.something they should use el.something. Again, this reinforces that el.you = 'set the value' is very bad.

Member

ibolmo commented Dec 11, 2011

The problem is that

el.anything will be considered an attribute by IE. This means that el.getAttribute('anything') will give the value of it.

One solution is to whitelist using setProperty any attributes and check against the property in getProperty if someone wants el.something they should use el.something. Again, this reinforces that el.you = 'set the value' is very bad.

ibolmo added a commit to ibolmo/mootools-core that referenced this issue Dec 12, 2011

Fixes #2109.
IE < 9, getAttribute returns properties set via `el.attribute = value`.
Other browsers return null for such properties that are not part of the
specs (e.g. `href`, `width`, or `title`).

This tries to normalize the behavior and prevents unwanted "expando"
properties from being returned (e.g. `inject` and other Element
methods).

TL;DR: whitelist any setProperty, and getProperty checks in the
whitelist or if the attribute is an expando (not part of the known
specs).

Fixed a spec with maxlenght property, when it needed to be maxlength.

PASSED: IE6-9; FFx 3-5, 8, 10, Opera 11, Chrome latest, Safari 5

@ibolmo ibolmo referenced this issue Dec 12, 2011

Merged

Fixes #2109. #2167

@ibolmo ibolmo closed this in 70b402e Jan 8, 2012

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