Reply to thread

Message: [QUOTE="Garrett Smith, post: 4995798"] [snip] I don't have time to review all the review, so I'll focus on a couple of things. First off, you did not format the review to wrap at 72 chars. The code is wrapping badly and much harder to read. The code looks it was originally well formatted. There are a number of problems in what little I've looked at. CSSOM Views Draft. [snip] Yep. Same with EXT-js. The Base.js file is full of logical errors, mistakes, browser detection. Had. There isn't anymore innerWidth there. The function in the FAQ works, but will probably fail in quirks mode for non-IE browsers. Design Issue: Their contains() method does not consider what happens for the case where target === element except for the case of document, where that case is false. Oddly, contains(document, null) will result true. Bug: They should not be using loose equality to compare input values. Strict equality, ===, should be used when comparing objects so that if one value happens to be primitive, there will not be a false positive. Bug: aNode.contains( aNode ); is going to be true in IE, Opera, and Safari 3, false in Firefox, false in Safari 2 and 3. Based on what they decided should be true for the case of document, it appears that contains(aNode, aNode) Without running the code, from memory, I know that Safari 2 and 3 implement a contains that returns false for aNode.contains( aNode ); Principle violation: Rely on standard features. Principle violation: Do not use faulty inferences (browser detection). The strategy uses browser detection to attempt to call a non-standard method. Browsers are famous for copying IE's APIs in desparate attempts to get sites working. The copied API is often a very poor knock off with different functionality. That's true here. In IE, aNode.contains(aNode) is going to be true. Opera copied that. Safari did not, but then changed in version 4. javascript: alert(document.body.contains(document.body)) Safari 2, 3: false Safari 4: true Opera 10: true IE (all): true Instead, the code should use feature detection to try to detect standard property compareDocumentPosition. Where a fallback is used, the fallback should take into account the intent of the code and address the deviations in contains so either IE or Safari 2 (Safari 2 lacks a compareDocumentPosition method), accounts for the case of element === target. The last function uses a while loop and performs a loose equality test. Instead, it should instead use strict equality. This last method will always return true when target === element. The method is intended to be used with a node that may be disconnected from the document. If - element - is disconnected from the document, then it should return false, otherwise it should return true. Bug: Using boolean conversion of - node.offsetParent - will result in error in MSIE when - node - is not part of the DOM. This the first case in the function, so it is highly likely that this method will throw errors in IE. javascript: alert(document.createElement("div").offsetParent) Bug: Using contains() will have different results, so document.body.contains(document.body) is going to be false in some browsers and true in others. Bug: document.body.contains(document.documentElement) will of course always be false! Misleading comment: // Contains() is only available on real elements in webkit and // not on the document. That is a true statement, but it is not a deviation in Webkit. Indeed, Opera, IE, and Webkit, document.contains is undefined. javascript: alert(document.contains); [/QUOTE]

Verification