Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

getAttribute on non-existent should return null for DOM4 and web compat #465

Closed
domenic opened this Issue Jul 3, 2012 · 5 comments

Comments

Projects
None yet
3 participants
Collaborator

domenic commented Jul 3, 2012

It turns out that, in all browsers since the IE6 days, getAttribute returns null instead of empty string on non-existent attributes. This was finally fixed in the DOM4 spec, despite being specified contrary to web compat all the way through DOM3.

I was going to fix this, as it was causing the Chai-jQuery tests to fail when run against jsdom. But, I wanted some input on how to do so. Should I try to create a level4 folder, with something that overrides the existing getAttribute?? Or should I patch the level1 implementation with a note in the source? Let me know and I'll put together a pull request.

Owner

tmpvar commented Jul 8, 2012

Should I try to create a level4 folder, with something that overrides the existing getAttribute

Yes. I'm actually surprised this hasn't been reported before. Do you know why Chai-jQuery is exploding?

Ideally the w3c has written a test for this and you can just port it over. Otherwise, we'll need a couple tests that reference that section of the spec and exercise all of the defined behavior.

I guess at that point, you'd also need to change the jsdom.defaultLevel so that jsdom.env uses it by default. I wonder if this will cause people's code to break.

Collaborator

domenic commented Jul 8, 2012

Do you know why Chai-jQuery is exploding?

Yeah, it's a bit weird, but Chai-jQuery checks strictly against undefined, and jQuery explicitly converts null to undefined but does not do the same for empty string. So it's kind of Chai-jQuery's fault for strictly checking against undefined, but kind of not, since in all browsers that works great.

Any hints on where to get W3C tests, since I guess you guys have done this before?

Owner

tmpvar commented Jul 8, 2012

Weird, a return (!!ret) ? undefined : ret would have the same effect. Oh, but if you have empty attrs (name="") then you have a side case.

When I initially collected the tests, I used : http://www.w3.org/DOM/Test/ and a little php script (grimace) https://github.com/tmpvar/jsdom/blob/master/test/collector.php in tandem with wget -s

So, since it looks like there isn't a test suite for dom level4 you may find something here: http://w3c-test.org/ or here https://www.google.com/search?q=site:w3.org+DOM4

The alternative is to write tests to test the hell out of it™

Collaborator

domenic commented Aug 25, 2012

I couldn't find any tests at w3c-test.org, so I asked the list. I also downloaded everything at http://w3c-test.org/html/tests/approved/ for good measure, but they don't seem to be in a collector.php-compatible format :(

However, note that making this change will cause level1 and level2 tests to fail. What should we do there? Remove them??

lchi commented Nov 1, 2012

This bug causes the version of Sizzle in jQuery at 1.7.2 to break because it explicitly checks against null for the existence of an attribute.

This is occurring at line 2560 in jquery.1.7.2.js.

domenic added a commit that referenced this issue Feb 6, 2013

NodeList; fixes #561.
NodeList is now always a NodeList, and never a NodeArray. It never gets array methods, for example. All of its internal methods have been underscore-prefixed. It also got a `constructor` property.

This broke #400 again, but this time the cause is #465.

@domenic domenic closed this in 80da721 Feb 6, 2013

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