Element.get not reading custom attributes in IE7/8 #2247

Closed
incendium opened this Issue Jan 23, 2012 · 6 comments

Comments

Projects
None yet
5 participants
@incendium

See the code below to reproduce the issue. I'll put a jsfiddle entry up when it's updated with 1.4.3.

Testcase:

<!DOCTYPE HTML>
<html lang="en-US">
  <head>
    <meta charset="UTF-8">
    <title>HTML 5 Test</title>

    <script type="text/javascript" src="mootools-1.4.3.js"></script>
    <script type="text/javascript">
      window.addEvent('domready', function() {
        var test = $('test');
        test.set('text', test.get('data-role') || 'null');
      });
    </script>
  </head>
  <body>
    <div id="test" data-role="test"></div>
  </body>
</html>

Chrome, IE9, and Firefox insert "test". IE7-8 insert "null".

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Jan 23, 2012

Member

You're right, also see http://jsfiddle.net/nbQEk/1/embedded/result/

@ibolmo any ideas what has changed there that could've caused this?

Member

arian commented Jan 23, 2012

You're right, also see http://jsfiddle.net/nbQEk/1/embedded/result/

@ibolmo any ideas what has changed there that could've caused this?

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Jan 23, 2012

Member

https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L629-632

I believe that IE is considering any non-normative attribute (data-*) as an
expando. Those lines were added to prevent custom methods from being
returned. Looks like my spec coverage was too simple.

For now you can workaround this with:

Element.Properties['data-role'] = {get: function(){
  return this.getAttribute('data-role');
}};

(I know) Not a great solution, since you'll need to repeat the above for
each custom property that you've set. But it's temporary for 1.4.4.

On Mon, Jan 23, 2012 at 11:54 AM, Arian Stolwijk <
reply@reply.github.com

wrote:

You're right, also see http://jsfiddle.net/nbQEk/1/embedded/result/

@ibolmo any ideas what has changed there that could've caused this?


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

Member

ibolmo commented Jan 23, 2012

https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L629-632

I believe that IE is considering any non-normative attribute (data-*) as an
expando. Those lines were added to prevent custom methods from being
returned. Looks like my spec coverage was too simple.

For now you can workaround this with:

Element.Properties['data-role'] = {get: function(){
  return this.getAttribute('data-role');
}};

(I know) Not a great solution, since you'll need to repeat the above for
each custom property that you've set. But it's temporary for 1.4.4.

On Mon, Jan 23, 2012 at 11:54 AM, Arian Stolwijk <
reply@reply.github.com

wrote:

You're right, also see http://jsfiddle.net/nbQEk/1/embedded/result/

@ibolmo any ideas what has changed there that could've caused this?


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

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Jan 23, 2012

Member

I'm going to get around to this one first thing tomorrow morning. I believe that adding a attr.specified check will alleviate the problem, but need to (thoroughly) test now.

Member

ibolmo commented Jan 23, 2012

I'm going to get around to this one first thing tomorrow morning. I believe that adding a attr.specified check will alleviate the problem, but need to (thoroughly) test now.

ibolmo added a commit to ibolmo/mootools-core that referenced this issue Jan 24, 2012

Fixes #2247.
This fixes a nasty regression that custom attributes set by HTML text
(e.g. innerHTML) would previously be considered `expando` and therefore
thought to have been considered a fake attribute. Therefore, we didn't
return the value.

This fix relies now on outerHTML to check for the existence of the
attribute. Keep in mind this also fixes the previous bug of returning
custom functions since any new `el.attribute =` are not shown in
outerHTML.

PASSED: IE6-9.

@ibolmo ibolmo closed this in d0901aa Jan 29, 2012

@incendium

This comment has been minimized.

Show comment
Hide comment
@incendium

incendium Feb 8, 2012

Working great now. Thanks for the quick turnaround.

Working great now. Thanks for the quick turnaround.

@DimitarChristoff

This comment has been minimized.

Show comment
Hide comment
@DimitarChristoff

DimitarChristoff Mar 15, 2012

Member

this is still not quite right. http://jsfiddle.net/dimitar/357Lt/ -> a data-propertyId is arguably wrong as HTML5 validation prefers all data- properties to be lowercase (W3C spec does not say anything in particular about their casing).

Browsers seem to adjust the element and fix attributes to lowercase but IE6/7/8 do not.

anyway: in IE7/8, .get('data-propertyid') - which is correct from a getter stand point as it SHOULD be lowercase - returns null though .getAttribute('data-propertyid') returns the value, despite of the element markup having data-propertyId.

basically, i guess I am saying: expect .get("data-Something") to return .getAttribute("data-Something"); in IE6/7/8 - or any camel cased / uppercased attribute, data or otherwise.

something that will probably fix it is:

https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L652 to convert to lowercase to match the whitelisted prop above at https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L648 and where it is being allowed https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L658

Member

DimitarChristoff commented Mar 15, 2012

this is still not quite right. http://jsfiddle.net/dimitar/357Lt/ -> a data-propertyId is arguably wrong as HTML5 validation prefers all data- properties to be lowercase (W3C spec does not say anything in particular about their casing).

Browsers seem to adjust the element and fix attributes to lowercase but IE6/7/8 do not.

anyway: in IE7/8, .get('data-propertyid') - which is correct from a getter stand point as it SHOULD be lowercase - returns null though .getAttribute('data-propertyid') returns the value, despite of the element markup having data-propertyId.

basically, i guess I am saying: expect .get("data-Something") to return .getAttribute("data-Something"); in IE6/7/8 - or any camel cased / uppercased attribute, data or otherwise.

something that will probably fix it is:

https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L652 to convert to lowercase to match the whitelisted prop above at https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L648 and where it is being allowed https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L658

@jabis

This comment has been minimized.

Show comment
Hide comment
@jabis

jabis Sep 12, 2013

In IE 7/8 This issue arose again on 1.4.5 ( build http://mootools.net/core/76bf47062d6c1983d66ce47ad66aa0e0)

jabis commented Sep 12, 2013

In IE 7/8 This issue arose again on 1.4.5 ( build http://mootools.net/core/76bf47062d6c1983d66ce47ad66aa0e0)

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