Skip to main content

Bug Tracker

Side navigation

#3763 closed bug (invalid)

Opened December 31, 2008 05:23PM UTC

Closed January 20, 2009 02:06AM UTC

attr() function's nodeType filtering isn't comprehensive

Reported by: amikitik Owned by:
Priority: minor Milestone: 1.3
Component: core Version: 1.2.6
Keywords: Cc:
Blocked by: Blocking:
Description

While this is likely a rare edge case, given the following code any attribute calls on generated DOM object (not pre-existing ones) will produce an error:

Object.prototype['testing'] = function (name) {
  var that = this,
    method = that[name];
  return function () { return method.apply(that, arguments); }
}
    
var b = $('<p>Example</p>');
b.css('display', 'block');

The fix for this is attached.

Attachments (1)
  • fix-attr.patch (0.4 KB) - added by amikitik December 31, 2008 05:23PM UTC.
Change History (3)

Changed December 31, 2008 05:30PM UTC by amikitik comment:1

Issue seems to still exist in the nightlies.

Changed December 31, 2008 06:44PM UTC by amikitik comment:2

While it's not smart to unconditionally add something to every object (like the function in the example), the code above exposes a potential problem.

Upon deeper investigation, jQuery uses 'in' loops quite a lot. If someone were to add something to the prototype of an object that jQuery loops over (say a function), then a loop over the elements could encounter unexpected type (string operations on a function). This could be avoided if every 'in' loop filtered the object with hasOwnProperty(). However, that seems like a performance penalty for a problem that really only manifests under unusual conditions (objects the get generated with additional prototype elements).

So file under "don't do that", I suppose? Or is there a built-in way to avoid this?

Changed January 20, 2009 02:06AM UTC by dmethvin comment:3

resolution: → invalid
status: newclosed

jQuery does not support changes to Object.prototype. The additional Object properties become visible to for-in loops and breaks any code that uses them.