Qooxdoo Reaches a Milestone! (1.0)

D

David Mark

More ace reporting from Ajaxian:-

http://ajaxian.com/archives/the-maturation-of-a-framework-qooxdoo-reaches-1-0

It's a 20MB download (no other option). I assume the bulk of that is
pretty graphics and useless tools (e.g. API viewer).

Doesn't take long to determine that the API is built on flyweight JS:-

/*
************************************************************************

qooxdoo - the new era of web development


More like another old time capsule of poor Web development practices
(would have been laughable at the turn of the century).



http://qooxdoo.org

Copyright:
2004-2008 1&1 Internet AG, Germany, http://www.1und1.de

License:
LGPL: http://www.gnu.org/licenses/lgpl.html
EPL: http://www.eclipse.org/org/documents/epl-v10.php
See the LICENSE file in the project's top-level directory for
details.

Authors:
* Sebastian Werner (wpbasti)


======================================================================

This class contains code based on the following work:

* Yahoo! UI Library
http://developer.yahoo.com/yui
Version 2.2.0


Oops.

[...]


************************************************************************
*/

/**
* Includes library functions to work with the client's viewport
(window).
*/


That's a pretty slap-dash description for a module in a "mature"
framework.


qx.Class.define("qx.bom.Viewport",
{
statics :
{
/**
* Returns the current width of the viewport (excluding a
eventually visible scrollbar).
*
* <code>clientWidth</code> is the inner width of an element in
pixels. It includes padding
* but not the vertical scrollbar (if present, if rendered),
border or margin.
*
* The property <code>innerWidth</code> is not useable as defined
by the standard as it includes the scrollbars


What standard?


* which is not the indented behavior of this method. We can
decrement the size by the scrollbar
* size but there are easier possibilities to work around this.

Yes, there are.


*
* Safari 2 and 3 beta (3.0.2) do not correctly implement
<code>clientWidth</code> on documentElement/body,


Sounds like dated observations.


* but <code>innerWidth</code> works there. Interesting is that
webkit do not correctly implement
* <code>innerWidth</code>, too. It calculates the size excluding
the scroll bars and this
* differs from the behavior of all other browsers - but this is
exactly what we want to have
* in this case.


A dated coincidence.


*
* Opera less then 9.50 only works well using
<code>body.clientWidth</code>.


Works well?


*
* Verified to correctly work with:
*
* * Mozilla Firefox 2.0.0.4
* * Opera 9.2.1
* * Safari 3.0 beta (3.0.2)
* * Internet Explorer 7.0


Definitely dated testing. But what were they testing anyway?


*
* @signature function(win)
* @param win {Window?window} The window to query
* @return {Integer} The width of the viewable area of the page
(excludes scrollbars).
*/
getWidth : qx.core.Variant.select("qx.client",
{
"opera" : function(win) {


They were testing if their UA sniffing was "accurate". :)


if (qx.bom.client.Engine.VERSION < 9.5) {
return (win||window).document.body.clientWidth;
}
else
{
var doc = (win||window).document;
return qx.bom.Document.isStandardMode(win) ?
doc.documentElement.clientWidth : doc.body.clientWidth;
}
},


Who wants to download this sort of bullshit (users or developers?)


"webkit" : function(win) {
if (qx.bom.client.Engine.VERSION < 523.15) { // Version <
3.0.4
return (win||window).innerWidth;
}
else
{


That's enough of that. It's another browser sniffing preserve.

http://www.cinsoft.net/viewport.asp

The FAQ has featured similar code for a decade.

And yes, the rest of it appears much the same:-

contains : qx.core.Variant.select("qx.client",
{
"webkit|mshtml|opera" : function(element, target)
{
if (qx.dom.Node.isDocument(element))
{
var doc = qx.dom.Node.getDocument(target);
return element && doc == element;
}
else if (qx.dom.Node.isDocument(target))
{
return false;
}
else
{
return element.contains(target);
}
},

// http://developer.mozilla.org/en/docs/DOM:Node.compareDocumentPosition
"gecko" : function(element, target) {
return !!(element.compareDocumentPosition(target) & 16);
},

"default" : function(element, target)
{
while(target)
{
if (element == target) {
return true;
}

target = target.parentNode;
}

return false;
}
}),


/**
* Whether the element is inserted into the document
* for which it was created.
*
* @param element {Element} DOM element to check
* @return {Boolean} <code>true</code> when the element is
inserted
* into the document.
*/
isRendered : function(element)
{
// This module is highly used by new qx.html.Element
// Copied over details from qx.dom.Node.getDocument() and
// this.contains() for performance reasons.

// Offset parent is a good start to test. It omits document
detection
// and function calls.
if (!element.offsetParent) {
return false;
}

var doc = element.ownerDocument || element.document;

// This is available after most browser excluding gecko haved
copied it from mshtml.
// Contains() is only available on real elements in webkit and
not on the document.
if (doc.body.contains) {
return doc.body.contains(element);
}

// Gecko way, DOM3 method
if (doc.compareDocumentPosition) {
return !!(doc.compareDocumentPosition(element) & 16);
}

// Should not happen :)
throw new Error("Missing support for isRendered()!");
},

Frameworks built on solid foundations mature, preserves just rot over
time.

And in the comments for the cited posting, somebody pointed out that
the demos are all blank pages with scripting disabled. A retort was:-

"I wonder what the intersection is between users who use the web
without JS turned on, and users who use the web wearing tin-foil hats…
hmmm"

If they only had a brain. :)
 
G

Garrett Smith

David said:
[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.
* The property <code>innerWidth</code> is not useable as defined
by the standard as it includes the scrollbars


What standard?
CSSOM Views Draft.

[snip]
Who wants to download this sort of bullshit (users or developers?)


"webkit" : function(win) {
if (qx.bom.client.Engine.VERSION < 523.15) { // Version <
3.0.4
return (win||window).innerWidth;
}
else
{


That's enough of that. It's another browser sniffing preserve.

Yep. Same with EXT-js. The Base.js file is full of logical errors,
mistakes, browser detection.
http://www.cinsoft.net/viewport.asp

Yep.

The FAQ has featured similar code for a decade.

Had. There isn't anymore innerWidth there. The function in the FAQ
works, but will probably fail in quirks mode for non-IE browsers.
And yes, the rest of it appears much the same:-

contains : qx.core.Variant.select("qx.client",
{
"webkit|mshtml|opera" : function(element, target)
{
if (qx.dom.Node.isDocument(element))
{
var doc = qx.dom.Node.getDocument(target);
return element && doc == element;
}
else if (qx.dom.Node.isDocument(target))
{
return false;
}
else
{
return element.contains(target);
}
},

// http://developer.mozilla.org/en/docs/DOM:Node.compareDocumentPosition
"gecko" : function(element, target) {
return !!(element.compareDocumentPosition(target) & 16);
},

"default" : function(element, target)
{
while(target)
{
if (element == target) {
return true;
}

target = target.parentNode;
}

return false;
}
}),

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.
/**
* Whether the element is inserted into the document
* for which it was created.
*
* @param element {Element} DOM element to check
* @return {Boolean} <code>true</code> when the element is
inserted
* into the document.
*/
isRendered : function(element)
{
// This module is highly used by new qx.html.Element
// Copied over details from qx.dom.Node.getDocument() and
// this.contains() for performance reasons.

// Offset parent is a good start to test. It omits document
detection
// and function calls.
if (!element.offsetParent) {
return false;
}

var doc = element.ownerDocument || element.document;

// This is available after most browser excluding gecko haved
copied it from mshtml.
// Contains() is only available on real elements in webkit and
not on the document.
if (doc.body.contains) {
return doc.body.contains(element);
}

// Gecko way, DOM3 method
if (doc.compareDocumentPosition) {
return !!(doc.compareDocumentPosition(element) & 16);
}

// Should not happen :)
throw new Error("Missing support for isRendered()!");
},

Frameworks built on solid foundations mature, preserves just rot over
time.

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);
 
G

Garrett Smith

Garrett said:
David said:

[snip]

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.

Ah, sorry, I'm not sure what:

qx.dom.Node.getDocument(target);

does, so that may or may not be true.

Their getDocument metho might throw error in that case. I can't say
because I did not look at it.
 
D

David Mark

Garrett said:
David said:
[snip]



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.

Ah, sorry, I'm not sure what:

   qx.dom.Node.getDocument(target);

does, so that may or may not be true.

Their getDocument metho might throw error in that case. I can't say
because I did not look at it.

/*

---------------------------------------------------------------------------
DOCUMENT ACCESS

---------------------------------------------------------------------------
*/

/**
* Returns the owner document of the given node
*
* @param node {Node|Document|Window} the node which should be
tested
* @return {Document|null} The document of the given DOM node
*/
getDocument : function(node)
{
return node.nodeType === this.DOCUMENT ? node : // is document
already
node.ownerDocument || // is DOM node
node.document; // is window
},

Worthless, "overloaded" nonsense. Takes elements, documents or
windows. What would be the point of passing a document?

But it's far better than the one that gets a window:-

/**
* Returns the DOM2 <code>defaultView</code> (window).
*
* @signature function(node)
* @param node {Node|Document|Window} node to inspect
* @return {Window} the <code>defaultView</code> of the given node
*/
getWindow : qx.core.Variant.select("qx.client",
{
"mshtml" : function(node)
{
// is a window already
if (node.nodeType == null) {
return node;
}


How can they beat this stuff out of keyboard without stopping to think
how stupid the underlying ideas are? And how do they figure they
won't have to come back and rewrite all of this crap as soon as
browsers change their respective tunes? From the comments, it's clear
nobody's home.


// jump to document
if (node.nodeType !== this.DOCUMENT) {
node = node.ownerDocument;
}

Ludicrous. If they want to know if that property exists...


// jump to window
return node.parentWindow;

This assumes that anything their browser sniffing identifies as IE
will support this property.


},

"default" : function(node)
{
// is a window already
if (node.nodeType == null) {
return node;
}


So nice, they wrote it twice. :)


// jump to document
if (node.nodeType !== this.DOCUMENT) {
node = node.ownerDocument;
}

And that.

// jump to window
return node.defaultView;


That may not be a window object at all, but that's a relatively
advanced topic. They'll never get there. ;)


}
}),

It makes me wonder if every contributor to an open source JS project
reads the same Browser Scripting for Morons book. How else to explain
all of these similarly awful projects, all making the same bonehead
mistakes? The clipboard? You'd think they could copy something
_recent_. :)

And there are still developers out there that swear - to this day -
that browser sniffing and other bizarre hacks are the only way to
write a framework. Similarly, they dismiss mountains of evidence to
the contrary as "academic" and endless examples of their incompetence
as "growing pains" (never mind that they are ten years behind).

I guess a lot of people see this stuff as impossible, despite the fact
that it's never been easier. Hanging (and hiding) behind the various
library "efforts" is a very hard way to go that is going to approach
impossible in the near future (jQuery is already there).
 
G

Garrett Smith

David said:
Garrett said:
David Mark wrote:
More ace reporting from Ajaxian:-
http://ajaxian.com/archives/the-maturation-of-a-framework-qooxdoo-rea... [snip]



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.
Ah, sorry, I'm not sure what:

qx.dom.Node.getDocument(target);

does, so that may or may not be true.

Their getDocument metho might throw error in that case. I can't say
because I did not look at it.

/*

---------------------------------------------------------------------------
DOCUMENT ACCESS

---------------------------------------------------------------------------
*/

/**
* Returns the owner document of the given node
*
* @param node {Node|Document|Window} the node which should be
tested
* @return {Document|null} The document of the given DOM node
*/
getDocument : function(node)
{
return node.nodeType === this.DOCUMENT ? node : // is document
already

A - this - reference in a static method. I should add that to my list of
problems in "coding standards". I can't remember seeing that problem
before, so it never crossed my mind.


....
getWindow : qx.core.Variant.select("qx.client",
{
"mshtml" : function(node)
{
// is a window already
if (node.nodeType == null) {
return node;
}

That's a bug. Anything without a nodeType is not necessarily the
parentWindow. It could be another window, iframe, etc.

]...]


[...]
// jump to document
if (node.nodeType !== this.DOCUMENT) {
node = node.ownerDocument;
}

That fails in IE 5.5, but probably not that big of a deal.

For IE 5.5, you need the document property: node.document.
And that.

// jump to window
return node.defaultView;


That may not be a window object at all, but that's a relatively
advanced topic. They'll never get there. ;)

In Safari 2, defaultView is not window.

Looks like the function does not accept null/undefined, so would result
in error. That is what I was wondering for the contains function. To be
fair, the method is documented as accepting:-
Node|Document|Window.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top