Skip to content

Commit

Permalink
Issue 5418 (DOM constants and properties are incorrectly recognized a…
Browse files Browse the repository at this point in the history 
…s user-defined properties)

http://code.google.com/p/fbug/issues/detail?id=5418
  • Loading branch information
@SebastianZ
SebastianZ committed Aug 29, 2012
1 parent f92e386 commit 5f24838
Showing 1 changed file with 156 additions and 8 deletions.
164 changes: 156 additions & 8 deletions extension/content/firebug/lib/dom.js
View file
Expand Up @@ -780,7 +780,7 @@ Dom.getDOMMembers = function(object)
else if (object instanceof Event || object instanceof Dom.EventCopy)
{ return domMemberCache.Event; }

return null;
return domMemberCache.Object;
};

Dom.isDOMMember = function(object, propName)
Expand All @@ -798,12 +798,26 @@ Dom.isDOMConstant = function(object, name)
if (name == "__proto__")
return false;

if (!(object instanceof Window ||
object instanceof Node ||
object instanceof Location ||
object instanceof Event ||
object instanceof Dom.EventCopy))
// object isn't recognized as such when using ===,
// so use this as workaround
isDOMProperty = (object.toString() == "[object Window]" || object.toString() == "[object Node]" ||

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Aug 29, 2012

Member 
var str = Object.prototype.toString.call(object);
var isDOMProperty = (str == "[object Window]" || ... );

should be faster, more reliable, and doesn't call user-defined functions.

This comment has been minimized.

Sign in to view

Copy link
@SebastianZ

SebastianZ Aug 29, 2012

Author Member

Ok, changed in 3294d09.

Sebastian
object.toString() == "[object Location]" || object.toString() == "[object Event]");

if (!(object === window.Window ||
object === window.Object ||
object === window.Node ||
object === window.Location ||
object === window.Event ||
object === Dom.EventCopy ||
object instanceof window.Window ||

This comment has been minimized.

Sign in to view

Copy link
@staabm

staabm Aug 29, 2012

object === window.Window and object instanceof window.Window evaluate to true for the same objects, doesn't it? (same for all other props which are checked here)

so just doing the instanceof check should be enough?

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Aug 29, 2012

Member

No - Window instanceof Window is false. However both === and instanceof look like they should be unnecessary because of isDOMProperty. (except EventCopy)

This comment has been minimized.

Sign in to view

Copy link
@SebastianZ

SebastianZ Aug 29, 2012

Author Member

However both === and instanceof look like they should be unnecessary because of isDOMProperty

Right. Though I don't have a clue why === is not working here. Executing Window === Window returns true but object === Window false. That's why I chose isDOMProperty as a (temporary) workaround.

Simon, any idea why?

Sebastian

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Aug 29, 2012

Member

Ah, yes, different windows. Then instanceof works only due to XPCOM magic.
isDOMProperty seems like a pretty good solution, actually.
object instanceof window.Node ||
object instanceof window.Location ||
object instanceof window.Event ||
object instanceof Dom.EventCopy ||
isDOMProperty))
{
return false;
}

return Dom.domConstantMap.hasOwnProperty(name);
}
Expand Down Expand Up @@ -856,6 +870,8 @@ domMemberMap.Window =
"mozPaintCount", //FF4.0
"mozRequestAnimationFrame", //FF4.0
"mozIndexedDB", //FF4.0
"mozCancelAnimationFrame",
"mozCancelRequestAnimationFrame",

"mozCancelAnimationFrame",
"mozCancelRequestAnimationFrame",
Expand Down Expand Up @@ -957,6 +973,133 @@ domMemberMap.Window =
"matchMedia", // https://developer.mozilla.org/en/DOM/window.matchMedia

"getInterface",

"BarProp",
"Controllers",
"Crypto",
"DOMException",
"DOMStringList",
"EventTarget",
"History",
"MimeTypeArray",
"MozURLProperty",
"Navigator",
"NodeList",
"OfflineResourceList",
"Screen",
"Storage",
"XULControllers",
"Document",
"Element",
"HTMLCollection",
"HTMLDocument",
"HTMLElement",
"HTMLHtmlElement",
"Infinity",
"JSON",
"Location",
"Math",
"NaN",
"Node",
"StopIteration",
"Window",
"XULElement",
"undefined",
"CSS2Properties",
"CSSStyleDeclaration",
"Error",
"EvalError",
"InternalError",
"Namespace",
"QName",
"RangeError",
"ReferenceError",
"SyntaxError",
"TypeError",
"URIError",
"Array",
"ArrayBuffer",
"Boolean",
"DataView",
"Date",
"Float32Array",
"Float64Array",
"Function",
"Int16Array",
"Int32Array",
"Int8Array",
"Iterator",
"Map",
"Number",
"Object",
"ParallelArray",
"QueryInterface",
"RegExp",
"Set",
"String",
"Uint16Array",
"Uint32Array",
"Uint8Array",
"Uint8ClampedArray",
"WeakMap",
"XML",
"XMLList",
"decodeURI",
"decodeURIComponent",
"dumpProfile",
"encodeURI",
"encodeURIComponent",
"escape",
"isFinite",
"isNaN",
"isXMLName",
"parseFloat",
"parseInt",
"pauseProfilers",
"resumeProfilers",
"startProfiling",
"stopProfiling",
"unescape",
"uneval"
];

domMemberMap.Object =
[
"arguments",
"caller",
"length",
"name",
"__defineGetter__",
"__defineSetter__",
"__lookupGetter__",
"__lookupSetter__",
"apply",
"bind",
"call",
"constructor",
"create",
"defineProperties",
"defineProperty",
"freeze",
"getOwnPropertyDescriptor",
"getOwnPropertyNames",
"getPrototypeOf",
"hasOwnProperty",
"isExtensible",
"isFrozen",
"isGenerator",
"isPrototypeOf",
"isSealed",
"keys",
"preventExtensions",
"propertyIsEnumerable",
"seal",
"toLocaleString",
"toSource",
"toString",
"unwatch",
"valueOf",
"watch"
];

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Aug 29, 2012

Member

Wait... This list is for Object, not for things inherited from Object.prototype...

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Jan 11, 2013

Member

Heh - I was just about to go here and point out the exact same thing again.

This comment has been minimized.

Sign in to view

Copy link
@fflorent

fflorent Jan 13, 2013

Member

Does it mean it should return the same array as Object.getOwnPropertyNames(Object) ? (Note: that's just another naive question)

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Jan 13, 2013

Member

The usage is approximately if (obj instanceof Object) return domMemberMap.Object, so clearly domMemberMap.Object was meant to contain everything on Object.prototype. But by mistake it contains everything on Object instead (which is totally unrelated).

This comment has been minimized.

Sign in to view

Copy link
@SebastianZ

SebastianZ Jan 14, 2013

Author Member

You missed to mention that you already removed domMemberMap.Object in ea6b354.

Sebastian

This comment has been minimized.

Sign in to view

Copy link
@simonlindholm

simonlindholm Jan 14, 2013

Member

True. I did make a note in the issue at least.

domMemberMap.Location =
Expand All @@ -972,7 +1115,9 @@ domMemberMap.Location =

"assign",
"reload",
"replace"
"replace",

"QueryInterface"
];

domMemberMap.Node =
Expand Down Expand Up @@ -1023,7 +1168,9 @@ domMemberMap.Node =
"isSupported",
"getFeature",
"getUserData",
"setUserData"
"setUserData",

"QueryInterface"
];

domMemberMap.Document = Arr.extendArray(domMemberMap.Node,
Expand Down Expand Up @@ -1909,6 +2056,7 @@ Dom.domInlineEventHandlersMap =
"onmozpointerlockchange": 1,
"onmozpointerlockerror": 1,
"onuserproximity": 1,
"onwheel": 1
}

// ********************************************************************************************* //
Expand Down

0 comments on commit 5f24838

Please sign in to comment.