D
David Mark
Sencha Touch is a bizarre mish-mash of ExtJS and a jQuery plug-in
called JQTouch. It is advertised as the first "HTML5 framework" based
on "standards" like HTML5 and CSS3. Of course, HTML5 is neither a
standard nor widely implemented and the script eschews CSS3 for
proprietary WebKit extensions. And the most bizarre thing is that
very little of the script relates to HTML5.
The script is touted as "cross-browser", despite the fact that it is
admittedly dual-browser at best. It weighs 228K (minified) and
several of its key features rely on UA-based browser sniffing.
The rationalization for these unfortunate facts is that Android and
iPhone/iPod/iPad devices account for 90% of the mobile market. It is
unclear who conducted that study; but regardless, unlike in school,
90% is not an A-, particularly when the remaining 10% are left with
non-functioning applications. The weight problem is dismissed with
the usual marketing tactic of quoting sizes after GZIP compression
(only 80K!) And feature detection is deemed "impossible" due to the
fact that common features implemented in the two "supported" browsers
vary in their behavior (apparently feature testing is outside of the
authors' realm of understanding).
Not much new here. It's the same "tired old arguments" that readers
of this group have heard over and over. This shouldn't be surprising
as library developers keep making the same tired old mistakes. And,
of course, most newcomers to this group have failed to read each and
every previous related post, so repetition is required.
/*
Copyright(c) 2010 Sencha Inc.
(e-mail address removed)
http://www.sencha.com/touchlicense
*/
Yes, they plan to charge money for this thing.
// for old browsers
window.undefined = window.undefined;
First line and it is the usual rookie mistake. Note that this line
runs in the global execution context, so - this - points to the global
object. Why not use that instead of attempting to augment a host
object? Likely because this line has been copied and pasted from a
similarly ludicrous script that preceded it. There is no standard for
the window object and, as a host object, it is explicitly allowed to
throw an exception (or fail silently) in this case.
This is not a minor quibble. IE can be configured to disallow host
object expandos and nobody knows how many other browsers behave in
similar fashion (perhaps even by default).
The sum effect of this first line of code is to indicate that the
authors don't really know what they are doing. Not only is "this"
shorter than "window" (neither of which will be shortened on
minification), but you have to wonder what "old browsers" this dual-
browser framework is attempting to placate. As will become evident,
no such browser stands a chance in hell of running this script, so the
only explanation is cargo cult programming (i.e. they saw this line in
another script and copied it without understanding the ramifications).
/**
* @class Ext
There are no classes in ECMAScript implementations.
* Ext core utilities and functions.
Everything old is new again.
* @singleton
It goes without saying that there are no singletons either.
*/
Ext.setup = function(config) {
if (Ext.isObject(config)) {
Oh dear. We'll get to that one shortly. Suffice to say that there is
no call for this (no pun intended).
if (config.addMetaTags !== false) {
var viewport = Ext.get(document.createElement('meta')),
app = Ext.get(document.createElement('meta')),
statusBar = Ext.get(document.createElement('meta')),
startupScreen =
Ext.get(document.createElement('link')),
appIcon = Ext.get(document.createElement('link'));
Okay. Five elements created.
viewport.set({
name: 'viewport',
content: 'width=device-width, user-scalable=no,
initial-scale=1.0, maximum-scale=1.0;'
});
No call for this either. As we'll see, the "set" function is another
botched attempt at setting attributes and/or properties.
if (config.fullscreen !== false) {
app.set({
name: 'apple-mobile-web-app-capable',
content: 'yes'
});
Ditto.
if (Ext.isString(config.statusBarStyle)) {
The isString function is also dubious.
statusBar.set({
name: 'apple-mobile-web-app-status-bar-style',
content: config.statusBarStyle
});
}
}
if (Ext.isString(config.tabletStartupScreen) &&
Ext.platform.isTablet) {
Ext.platform is populated largely by UA-based browser sniffing.
startupScreen.set({
rel: 'apple-touch-startup-image',
href: config.tabletStartupScreen
});
} else if (Ext.isString(config.phoneStartupScreen) &&
Ext.platform.isPhone) {
startupScreen.set({
rel: 'apple-touch-startup-image',
href: config.phoneStartupScreen
});
}
if (config.icon) {
config.phoneIcon = config.tabletIcon = config.icon;
}
This is a very odd design. Why have three properties? If they were
going to allow specifying separate icons for what they "detect" as
phones and tablets, then surely they shouldn't step on them without
looking.
var precomposed = (config.glossOnIcon == false) ? '-
precomposed' : '';
if (Ext.isString(config.tabletIcon) &&
Ext.platform.isTablet) {
Why didn't they just check config.icon?
appIcon.set({
rel: 'apple-touch-icon' + precomposed,
href: config.tabletIcon
});
} else if (Ext.isString(config.phoneIcon) &&
Ext.platform.isPhone) {
appIcon.set({
el: 'apple-touch-icon' + precomposed,
href: config.phoneIcon
});
}
var head = Ext.get(document.getElementsByTagName('head')
[0]);
Why pass the result to Ext.get? Is there some ill-advised host object
augmentation going on here?
head.appendChild(viewport);
if (app.getAttribute('name')) head.appendChild(app);
if (statusBar.getAttribute('name'))
head.appendChild(statusBar);
if (appIcon.getAttribute('href'))
head.appendChild(appIcon);
if (startupScreen.getAttribute('href'))
head.appendChild(startupScreen);
}
Nope. They used only standard DOM methods. Of course, there was no
need to call getAttribute at all. They could have just checked the
corresponding DOM properties; but more importantly, as seen above,
they are the ones who set (or didn't set) these attributes in the
first place. In other words, the logic takes the long way around,
unnecessarily involving one of the least trustworthy methods in the
history of browser scripting (getAttribute).
And why did they create all of those elements in advance when some or
all of them may not be appended at all? In short, the whole preceding
mess could be re-factored in five minutes to save several function and
host method calls, not to mention the creation of up to five elements.
When choosing a browser script, one of the first questions should be
who: wrote it and what is their relative level of proficiency? It's
not a "personal smear" to make a judgment call at this point. The
authors are obviously yet another batch of clueless neophytes (see
also jQuery, Prototype, Dojo, YUI, ExtJS, MooTools, etc.) Suffice to
say that leaning on a script written by such authors is going to lead
to problems. Readers with even the slightest experience in cross-
browser scripting can stamp this one "Avoid at all Costs" and move on
at this point (if they haven't already).
if (Ext.isArray(config.preloadImages)) {
Oops. There is no way to write a reliable "isArray" function in JS.
And as above, there is no reason to do anything more than a boolean
type conversion here (as long as the documentation indicates that this
property must be an array). Anything "truthy" that is not an array
will result in a silent failure as written, which is the least helpful
result (often described as "robustness" by library authors).
for (var i = config.preloadImages.length - 1; i >= 0; i--)
{
How about:-
for (var i = config.preloadImages.length; i-- {
Sure that's a minor quibble, but it is yet another glimpse into the
authors' proficiency (or lack thereof). We are only a few dozen lines
in and virtually every line needs to be rewritten (which should take
*one* proficient JS developer about ten minutes).
(new Image()).src = config.preloadImages;
};
}
if (Ext.isFunction(config.onReady)) {
Ext.onReady(config.onReady, config.scope || window);
}
As we'll see, the isFunction function is yet another dubious entry.
And scope has *nothing* to do with the - this - despite the insistence
of seemingly every "major" library author. In this case, it's not
just a naming snafu as the ExtJS developers constantly refer to - this
- as "scope" in their documentation and support forums. And if they
don't understand why that is wrong, they haven't gotten past the first
page of the manual. I wonder what they call scope. Bananas?
Seriously, you have to wonder if these developers understand the
language they are using at all.
Ext.apply = function(object, config, defaults) {
// no "this" reference for friendly out of scope calls
There they go again.
if (defaults) {
What? No isObject call?
Ext.apply(object, defaults);
}
if (object && config && typeof config == 'object') {
for (var key in config) {
Oops. Unfiltered for-in loops are a very bad idea (at least for
scripts deployed on the Web). If this script is mashed up with
something that augments Object.prototype (e.g. older versions of the
unfortunately named Prototype library), all hell will break loose here
(in the deepest part of their core).
object[key] = config[key];
}
}
return object;
};
Calling this function "apply" was a pretty silly idea as well (because
of Function.prototype.apply). You've got to wonder if the authors are
that out of touch (no pun intended) or simply trying to confuse
beginners to keep them dependent on their dubious offerings. Both are
unsavory prospects.
Ext.apply(Ext, {
userAgent: navigator.userAgent.toLowerCase(),
Nellie bar the door. As has been known for a decade, referencing this
deception device cannot lead to anything good.
applyIf : function(object, config) {
var property, undefined;
There's no need to declare a local "undefined" identifier.
if (object) {
for (property in config) {
Another unfiltered for-in.
if (object[property] === undefined) {
Just use the typeof operator.
object[property] = config[property];
}
}
}
return object;
},
/**
* Repaints the whole page. This fixes frequently encountered
painting issues in mobile Safari.
*/
Their "fix" is nothing but a mystical incantation. Clearly their
script has had problems with the one platform they seek to support,
but rather than attempting to understand the cause of these problems,
they've resorted to nonsense code that appears to "fix" the problem in
whatever mobile devices they had handy to test with at the time of
writing.
repaint : function() {
var mask = Ext.getBody().createChild({
cls: 'x-mask x-mask-transparent'
});
Here they create and immediately discard a wrapper object for
document.body.
setTimeout(function() {
mask.remove();
}, 0);
ISTM that using 0 for the second argument to setTimeout is ill-advised
(as is the implied global).
},
/**
* Generates unique ids. If the element already has an id, it is
unchanged
* @param {Mixed} el (optional) The element to generate an id for
* @param {String} prefix (optional) Id prefix (defaults "ext-
gen")
* @return {String} The generated Id.
*/
id : function(el, prefix) {
return (el = Ext.getDom(el) || {}).id = el.id || (prefix ||
"ext-gen") + (++Ext.idSeed);
},
That's just plain ridiculous and falls under the category of "concise"
code that is all the rage these days. How about something like this:-
var id;
if (typeof el == 'string') { // ID passed, find it
el = document.getElementById(el);
}
if (el) { // Element exists
if (el.id) { // Has an ID
id = el.id;
} else { // Does not have an ID, assign
id = el.id = (prefix || "ext-gen") + (++Ext.idSeed);
}
}
return id; // undefined if element not found
It's not as if that will be any longer once white space and comments
are removed. Will be a hell of a lot easier to maintain and debug as
well (no phantom ID's generated).
[skipped tired old Ext "class" related functions]
urlEncode : function(o, pre){
var empty,
buf = [],
e = encodeURIComponent;
Ext.iterate(o, function(key, item){
Why not just use a for-in loop? If the (omitted) comments are to be
believed, then o is only allowed to be an Object object. The iterate
function makes calls to isObject, isEmpty and isIterable in a futile
attempt to support Object, Array and host objects with one function.
empty = Ext.isEmpty(item);
As we shall see, isEmpty is another unnecessary function and itself
calls isArray (of all things).
Ext.each(empty ? key : item, function(val){
buf.push('&', e(key), '=', (!Ext.isEmpty(val) && (val !
= key || !empty)) ? (Ext.isDate(val) ? Ext.encode(val).replace(/"/g,
'') : e(val)) : '');
});
});
Hmmm. Skipping ahead, the Ext.encode function is defined as:-
/**
* Shorthand for {@link Ext.util.JSON#encode}
* @param {Mixed} o The variable to encode
* @return {String} The JSON string
* @member Ext
* @method encode
*/
Ext.encode = Ext.util.JSON.encode;
And the "longhand" version is defined as:-
/**
* @class Ext.util.JSON
How is an object literal a class?
* Modified version of Douglas Crockford"s json.js that doesn"t
* mess with the Object prototype
* http://www.json.org/js.html
* @singleton
*/
Ext.util.JSON = {
encode : function(o) {
return JSON.stringify(o);
},
Now what does JSON have to do with urlencoding dates?
So far none of this is anything close to professional code and much of
it is positively senseless. Still no sign of HTML5 either.
On to decoding:-
/**
* Takes an encoded URL and and converts it to an object. Example:
* <pre><code>
Ext.urlDecode("foo=1&bar=2"); // returns {foo: "1", bar: "2"}
Ext.urlDecode("foo=1&bar=2&bar=3&bar=4", false); // returns {foo: "1",
bar: ["2", "3", "4"]}
</code></pre>
Of course, none of those are URL's.
* @param {String} string
* @param {Boolean} overwrite (optional) Items of the same name
will overwrite previous values instead of creating an an array
(Defaults to false).
* @return {Object} A literal with members
A what?!
*/
urlDecode : function(string, overwrite){
if(Ext.isEmpty(string)){
return {};
}
var obj = {},
pairs = string.split('&'),
d = decodeURIComponent,
name,
value;
Ext.each(pairs, function(pair) {
pair = pair.split('=');
name = d(pair[0]);
value = d(pair[1]);
obj[name] = overwrite || !obj[name] ? value :
[].concat(obj[name]).concat(value);
});
return obj;
},
Interesting. No call to Ext.decode. I guess JSON is only related to
encoding URL's.
/**
* Convert certain characters (&, <, >, and ') to their HTML
character equivalents for literal display in web pages.
* @param {String} value The string to encode
* @return {String} The encoded text
*/
htmlEncode : function(value){
return Ext.util.Format.htmlEncode(value);
},
Unnecessary bloat and another unneeded function call. Odd choices for
a mobile framework.
/**
* Appends content to the query string of a URL, handling logic
for whether to place
* a question mark or ampersand.
* @param {String} url The URL to append to.
* @param {String} s The content to append to the URL.
* @return (String) The resulting URL
*/
urlAppend : function(url, s){
if(!Ext.isEmpty(s)){
Another half-dozen unneeded function calls. This would be bad enough
in and of itself, but the functions in question are highly dubious as
well. Try this:-
if (s) {
return url + (url.indexOf('?') === -1 ? '?' : '&') + s;
No need for a strict comparison there. It's ham-fisted and makes
future maintainers (and reviewers) have to pause and wonder why they
did it.
}
return url;
},
/**
* Converts any iterable (numeric indices and a length property)
into a true array
There's no such thing as an "iterable" or a "true array".
* Don't use this on strings. IE doesn't support "abc"[0] which
this implementation depends on.
Oh, I they needn't worry about IE.
* For strings, use this instead: "abc".match(/./g) => [a,b,c];
That's a syntax error (so don't use it).
* @param {Iterable} the iterable object to be turned into a true
Array.
* @return (Array) array
*/
toArray : function(array, start, end){
return Array.prototype.slice.call(array, start || 0, end ||
array.length);
That will blow up in IE < 9.
},
/**
* Iterates an array calling the supplied function.
Not according to the next line.
* @param {Array/NodeList/Mixed} array The array to be iterated.
If this
There is no way to reliably differentiate between Array and host
objects (so don't design systems that hinge on making that work). And
I don't know what a "Mixed" is supposed to be.
each : function(array, fn, scope) {
That's not scope!
if (Ext.isEmpty(array, true)) {
return 0;
}
Whatever. They'd have been much better off copying
Array.prototype.forEach (then they could use that when available).
if (!Ext.isIterable(array) || Ext.isPrimitive(array)) {
array = [array];
}
Now there's a bizarre disjunction. If it is not "iterable" or it is a
primitive? You have to wonder what sort of "iterable" their code
would identify as a primitive. It's positively Dojo-esque (meaning
full of incomprehensible intertwined type-checking calls where none
are needed).
for (var i = 0, len = array.length; i < len; i++) {
if (fn.call(scope || array, array, i, array) ===
false) {
Why would they set - this - to array? That's going to lead to some
odd results.
return i;
};
}
return true;
},
iterate : function(obj, fn, scope){
if(Ext.isEmpty(obj)){
return;
}
if (Ext.isIterable(obj)) {
Ext.each(obj, fn, scope);
return;
}
else if(Ext.isObject(obj)) {
for (var prop in obj) {
if (obj.hasOwnProperty(prop)) {
Finally, a filter! Inconsistent with the rest though. You've got
to wonder if they simply forgot to filter the others. More likely
this is simply a slap-dash patchwork of previous scripts (e.g. ExtJS,
JQTouch).
if (fn.call(scope || obj, prop, obj[prop], obj)
=== false) {
return;
};
}
}
}
},
The indisputedly unreliable (and wholly unneeded) type-checking
permeates everything. All hopes of cross-browser compatibility are
lost (for no other reason than the authors were/are too green to
design a system this complicated).
* <b>Note</b>: the dom node to be found actually needs to exist
(be rendered, etc)
* when this method is called to be successful.
* @param {Mixed} el
Apparently "Mixed" means "botched". In this case, the function is
expected to discriminate between native and host objects. Such
"overloading" strategies are highly ill-advised in JS, yet seemingly
everything in this script relies on them.
* @return HTMLElement
*/
getDom : function(el) {
if (!el || !document) {
return null;
}
Now when is document going to type-convert to false?
return el.dom ? el.dom : (typeof el == 'string' ?
document.getElementById(el) : el);
},
/**
* Returns the current document body as an {@link Ext.Element}.
* @return Ext.Element The document body
*/
getBody : function(){
return Ext.get(document.body || false);
},
More gobbledygook. It's as if every line is designed with madness in
mind. The first line of Ext.get is:-
if(!el){
return null;
}
....and it's not as if document.body will be missing in either of the
two browsers they aspire to support. Regardless, why not something
like this:-
var body = document.body;
return body ? Ext.get(body) : null;
Function calls are not free and a "mobile framework" needs to be as
efficient as possible due to the limited resources available to mobile
browsers. By the same token, property access costs and this thing
uses them constantly when they could easily be avoided.
/**
* Returns the current HTML document object as an {@link
Ext.Element}.
* @return Ext.Element The document
*/
getDoc : function(){
return Ext.get(document);
},
This is jQuery-itis. In other words, an API abstraction that makes no
sense. Why would they wrap a document object in an Ext.Element
object. After all, elements and documents are very different types of
DOM nodes. What do the element-specific methods do for documents?
And assuming there are any document-specific methods, what do they do
for elements? It's mind-boggling. The only rationalization I've
heard for such a bizarre design is that Web developers could be
confused by more than one wrapper type.
/**
* This is shorthand reference to {@link Ext.ComponentMgr#get}.
* Looks up an existing {@link Ext.Component Component} by {@link
Ext.Component#id id}
* @param {String} id The component {@link Ext.Component#id id}
* @return Ext.Component The Component, <tt>undefined</tt> if not
found, or <tt>null</tt> if a
* Class was found.
*/
getCmp : function(id){
return Ext.ComponentMgr.get(id);
},
Another ridiculous waste of time and space.
So far, everything that can go wrong has gone wrong. Literally. No
doubt, this is somehow my fault. In other words, if I hadn't pointed
out all of these problems then they wouldn't exist.
/**
* Returns the current orientation of the mobile device
* @return {String} Either 'portrait' or 'landscape'
*/
getOrientation: function() {
return window.innerHeight > window.innerWidth ? 'portrait' :
'landscape';
},
Highly dubious and virtually never needed. In the same way that older
libraries attempt to replace rock-solid mechanisms like IE's
conditional comments with browser sniffing, this attempts to can CSS3
media queries.
http://www.w3.org/TR/css3-mediaqueries/#orientation
Of course, you can't sell what you can't can.
/**
* <p>Removes this element from the document, removes all DOM
event listeners, and deletes the cache reference.
* All DOM event listeners are removed from this element. If
{@link Ext#enableNestedListenerRemoval} is
* <code>true</code>, then DOM event listeners are also removed
from all child nodes. The body node
* will be ignored if passed in.</p>
* @param {HTMLElement} node The node to remove
*/
removeNode : function(n){
if (n && n.parentNode && n.tagName != 'BODY') {
Checking for the body is ludicrous. If the calling application fouls
up and passes the body, a silent failure will only obscure the
mistake, making it harder for the developers to track it down. Same
for checking the parentNode property.
Ext.EventManager.removeAll(n);
Almost certainly unneeded. If their EventManager thing actually
creates circular references then they should fix that, not dance
around the problem with this outdated Crockfordian strategy. And
regardless, this script won't work with IE (the browser with the
circular reference problem) anyway.
n.parentNode.removeChild(n);
delete Ext.cache[n.id];
}
},
/**
* Attempts to destroy any objects passed to it by removing all
event listeners, removing them from the
* DOM (if applicable) and calling their destroy functions (if
available). This method is primarily
* intended for arguments of type {@link Ext.Element} and {@link
Ext.Component}, but any subclass of
There are no classes in JS, so there can be no subclasses.
* {@link Ext.util.Observable} can be passed in. Any number of
elements and/or components can be
* passed into this function in a single call as separate
arguments.
* @param {Mixed} arg1 An {@link Ext.Element}, {@link
Ext.Component}, or an Array of either of these to destroy
Again, you cannot reliably tell an Object from an Array object.
Designing such systems in JS is like deliberately walking into a wall
(over and over in the case of this script).
* @param {Mixed} arg2 (optional)
* @param {Mixed} etc... (optional)
*/
destroy : function() {
var ln = arguments.length,
i, arg;
for (i = 0; i < ln; i++) {
arg = arguments;
if (arg) {
if (Ext.isArray(arg)) {
this.destroy.apply(this, arg);
}
else if (Ext.isFunction(arg.destroy)) {
arg.destroy();
}
else if (arg.dom) {
arg.remove();
}
}
}
},
isIterable : function(v){
//check for array or arguments
if(Ext.isArray(v) || v.callee){
return true;
}
The isArray function is unreliable and the presence of a "callee"
property does not indicate anything close to what they are trying to
determine (see their previous definition of an "iterable"). And
you've got to wonder what sort of strange design would require passing
the arguments object to a function like this. I mean, the only way a
variable could reference an arguments objects is by explicitly setting
it to reference an arguments object.
//check for node list type
if(/NodeList|
HTMLCollection/.test(Object.prototype.toString.call(v))){
return true;
}
Not only is the RegExp botched, but host objects are allowed to return
*anything* in response to a toString call. For example, this will
fail in many versions of IE (and presumably at least some of its
mobile derivations). Of course, this script will fall flat on its
face in IE anyway, for no reason other than dubious design decisions.
//NodeList has an item and length property
//IXMLDOMNodeList has nextNode method, needs to be checked
first.
Suffice to say that an application that needs to pass an XML nodelist
to this function is doomed from the start. Likely that includes the
framework itself.
return ((typeof v.nextNode != 'undefined' || v.item) &&
Ext.isNumber(v.length));
},
Ext.isNumber is another one of the botched (and unneeded) type-
checking functions.
So obviously, the "isIterable" function is an untenable (and unneeded)
design. And each time the authors hit upon a host object that
returned the "wrong" result they added another set of checks to make
it "right", instead of realizing they were pissing in the wind from
the start. Again, very Dojo-esque.
/**
* Utility method for validating that a value is numeric,
returning the specified default value if it is not.
* @param {Mixed} value Should be a number, but any type will be
handled appropriately
* @param {Number} defaultValue The value to return if the
original value is non-numeric
* @return {Number} Value, if numeric, else defaultValue
*/
num : function(v, defaultValue){
v = Number(Ext.isEmpty(v) || Ext.isArray(v) || typeof v ==
'boolean' || (typeof v == 'string' && v.trim().length == 0) ? NaN :
v);
return isNaN(v) ? defaultValue : v;
},
Another faulty design, rendered in typically bizarre and unreliable
fashion (nothing that calls isArray can be considered reliable). And
you really must wonder what sort of application would need such a
function.
/**
* <p>Returns true if the passed value is empty.</p>
* <p>The value is deemed to be empty if it is<div class="mdetail-
params"><ul>
* <li>null</li>
* <li>undefined</li>
* <li>an empty array</li>
* <li>a zero length string (Unless the <tt>allowBlank</tt>
parameter is <tt>true</tt>)</li>
* </ul></div>
* @param {Mixed} value The value to test
* @param {Boolean} allowBlank (optional) true to allow empty
strings (defaults to false)
* @return {Boolean}
*/
isEmpty : function(v, allowBlank) {
return v == null || ((Ext.isArray(v) && !v.length)) || (!
allowBlank ? v === '' : false);
},
Another call to isArray, which means the previous functions results in
two calls to that unreliable function. There appears to be no real
design work here, just a bunch of ill-advised functions tangled up in
haphazard fashion. I can hear the chorus of "show me where it fails"
now.
And again, read the description and wonder what sort of strange
application would need such a function.
/**
* Returns true if the passed value is a JavaScript array,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isArray : function(v) {
return Object.prototype.toString.apply(v) === '[object
Array]';
},
There it is. The "Miller Device" has been proven unreliable.
/**
* Returns true if the passed object is a JavaScript date object,
otherwise false.
* @param {Object} object The object to test
* @return {Boolean}
*/
isDate : function(v) {
return Object.prototype.toString.apply(v) === '[object Date]';
},
Same.
/**
* Returns true if the passed value is a JavaScript Object,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isObject : function(v) {
return !!v && Object.prototype.toString.call(v) === '[object
Object]';
},
The left part of the conjunction is clearly unneeded. And if they
would just stop to think about their design, they'd realize that a
typeof test would do just as well if they disallowed host objects as
arguments to this function (which they should do anyway).
/**
* Returns true if the passed value is a JavaScript 'primitive', a
string, number or boolean.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isPrimitive : function(v) {
return Ext.isString(v) || Ext.isNumber(v) || Ext.isBoolean(v);
},
This review could really be compressed to a string of "unneeded and
botched" comments (a good ways through the core of this thing nothing
is close to competently designed or implemented). Trying to learn
anything about browser scripting from the code and/or documentation of
scripts like this is hopeless as the authors mangle virtually every
aspect of the language (e.g. scope, primitives, literals, etc.) Yet
somehow many developers have come to the conclusion that they *must*
abdicate all responsibility for browser scripting to "expert" efforts
like this one.
/**
* Returns true if the passed value is a JavaScript Function,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isFunction : function(v) {
return Object.prototype.toString.apply(v) === '[object
Function]';
},
Unneeded and botched. The ambiguities that led to the advent of the
(unreliable) "Miller Device" do not apply to Function objects (i.e.
just use typeof). But I suppose the "designers" wanted to allow for
discrimination between RegExp, Function and host objects, despite the
fact that there is no practical application for such testing.
Ironically, defenders of such scripts often pepper their retorts with
references to the "real world".
/**
* Returns true if the passed value is a number. Returns false for
non-finite numbers.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isNumber : function(v) {
return Object.prototype.toString.apply(v) === '[object
Number]' && isFinite(v);
},
Unneeded and botched. Recall that isPrimitive calls this, yet it is
clearly "designed" to allow Number objects to slip through.
/**
* Returns true if the passed value is a string.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isString : function(v) {
return Object.prototype.toString.apply(v) === '[object
String]';
},
Unneeded and botched (see previous).
/**util
* Returns true if the passed value is a boolean.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isBoolean : function(v) {
return Object.prototype.toString.apply(v) === '[object
Boolean]';
},
Clearly we are dealing with a cargo cult here, copying and pasting
patterns of code without the slightest understanding of what it does.
This one reduces to:-
isBoolean : function(v) {
return typeof v == 'boolean';
},
....which is clearly a waste of a function call (why wouldn't you just
use the typeof operator in the first place?)
/**
* Returns true if the passed value is an HTMLElement
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isElement : function(v) {
return !!v && v.tagName;
},
Unneeded and botched.
/**
* Returns true if the passed value is not undefined.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isDefined : function(v){
return typeof v !== 'undefined';
},
Unneeded function with an unneeded strict comparison.
/**
* URL to a blank file used by Ext when in secure mode for iframe src
and onReady src to prevent
* the IE insecure content warning (<tt>'about:blank'</tt>, except for
IE in secure mode, which is <tt>'javascript:""'</tt>).
* @type String
*/
Ext.SSL_SECURE_URL = Ext.isSecure && 'about:blank';
Huh? This is isSecure:-
isSecure : /^https/i.test(window.location.protocol),
....so if the document is not served with the https protocol, the above
will be false (which is obviously not a string).
Ext.ns = Ext.namespace;
Why? To save themselves keystrokes? Or perhaps this is a misguided
attempt to shorten the download. Either way it makes no sense.
I'm still looking for the first bit that indicates some semblance of
sense. Though I know there are those who will dismiss this review in
light of the really "cool" Solitaire demo. In other words, regardless
of how bad the code is, if any sort of application can be demonstrated
to "work" in at least two browsers, then arguments about the quality
of the code can be dismissed out of hand (commonly phrased as "we
write code for users!").
Ext.ns(
'Ext.util',
'Ext.data',
'Ext.list',
'Ext.form',
'Ext.menu',
'Ext.state',
'Ext.layout',
'Ext.app',
'Ext.ux',
'Ext.plugins',
'Ext.direct'
);
Another waste of time and space, though I am sure some misguided
programmers would consider this more "advanced" than simple assigning
properties to the Ext object.
I skipped the "namespace" function; here it is:-
/**
* Creates namespaces to be used for scoping variables and classes
so that they are not global.
Scoping?! This (repeated) criticism is not being petty. I run into
programmers all the time who use the term "scope" to refer to all
sorts of things that have nothing to do with scope. I have to wonder
what word they would use if they actually wanted to discuss scope
(it's not as if scope is an arcane subject in the context of JS).
Scripts and comments like this are to blame.
* Specifying the last node of a namespace implicitly creates all
other nodes. Usage:
* <pre><code>
Ext.namespace('Company', 'Company.data');
Ext.namespace('Company.data'); // equivalent and preferable to above
syntax
Company.Widget = function() { ... }
Company.data.CustomStore = function(config) { ... }
</code></pre>
* @param {String} namespace1
* @param {String} namespace2
* @param {String} etc
* @return {Object} The namespace object. (If multiple arguments
are passed, this will be the last namespace created)
* @method namespace
*/
namespace : function() {
var ln = arguments.length,
i, value, split, x, xln;
for (i = 0; i < ln; i++) {
value = arguments;
parts = value.split(".");
object = window[parts[0]] = Object(window[parts[0]]);
As mentioned, using the window object in lieu of a reference to the
global object is a common rookie mistake.
And why are they calling Object? I suppose it is in the name of
"robustness" again, obscuring rather than exposing problems and making
the debugging of applications more difficult.
For example, assume that the global "Ext" variable unexpectedly has
been set to the primitive value of 5. Passing that to Object will
result in a Number object, not an Object object (clearly not what was
desired). The application developer should be aware that something
had gone horribly wrong prior to the conversion, so forcing the issue
is counter-productive.
for (x = 1, xln = parts.length; x < xln; x++) {
object = object[parts[x]] = Object(object[parts[x]]);
More of the same.
}
}
return object;
},
[Skip the usual ExtJS function wrapper suspects]
/**
* @class String
* These functions are available on every String object.
*/
Ext.applyIf(String.prototype, {
Good night.
Never did get to the HTML5 bits. Having reviewed the script in its
entirety for a client, I know they are few and far between. There's a
dozen line function to create AUDIO elements (complete with browser
sniffing and lacking any sort of feature detection), and another that
does the same for VIDEO elements. I don't know if localStorage is
part of the "official" HTML5 effort, but they've got a wrapper for
that. Oh and another dozen lines dealing with geolocation (also fuzzy
on whether that is part of HTML5). IIRC, that's about it.
As for the "touch" part. They've attempted to smooth out the
differences in the ontouch* event handling (between the two browsers
they aspire to support) using more browser sniffing. Also IIRC, they
attempt to synthesize the ongesture* events based on touches. Like
the rest of it, it's a tangled up house of cards.
Pretty lousy name too. Sencha doesn't exactly roll off the tongue,
does it?
called JQTouch. It is advertised as the first "HTML5 framework" based
on "standards" like HTML5 and CSS3. Of course, HTML5 is neither a
standard nor widely implemented and the script eschews CSS3 for
proprietary WebKit extensions. And the most bizarre thing is that
very little of the script relates to HTML5.
The script is touted as "cross-browser", despite the fact that it is
admittedly dual-browser at best. It weighs 228K (minified) and
several of its key features rely on UA-based browser sniffing.
The rationalization for these unfortunate facts is that Android and
iPhone/iPod/iPad devices account for 90% of the mobile market. It is
unclear who conducted that study; but regardless, unlike in school,
90% is not an A-, particularly when the remaining 10% are left with
non-functioning applications. The weight problem is dismissed with
the usual marketing tactic of quoting sizes after GZIP compression
(only 80K!) And feature detection is deemed "impossible" due to the
fact that common features implemented in the two "supported" browsers
vary in their behavior (apparently feature testing is outside of the
authors' realm of understanding).
Not much new here. It's the same "tired old arguments" that readers
of this group have heard over and over. This shouldn't be surprising
as library developers keep making the same tired old mistakes. And,
of course, most newcomers to this group have failed to read each and
every previous related post, so repetition is required.
/*
Copyright(c) 2010 Sencha Inc.
(e-mail address removed)
http://www.sencha.com/touchlicense
*/
Yes, they plan to charge money for this thing.
// for old browsers
window.undefined = window.undefined;
First line and it is the usual rookie mistake. Note that this line
runs in the global execution context, so - this - points to the global
object. Why not use that instead of attempting to augment a host
object? Likely because this line has been copied and pasted from a
similarly ludicrous script that preceded it. There is no standard for
the window object and, as a host object, it is explicitly allowed to
throw an exception (or fail silently) in this case.
This is not a minor quibble. IE can be configured to disallow host
object expandos and nobody knows how many other browsers behave in
similar fashion (perhaps even by default).
The sum effect of this first line of code is to indicate that the
authors don't really know what they are doing. Not only is "this"
shorter than "window" (neither of which will be shortened on
minification), but you have to wonder what "old browsers" this dual-
browser framework is attempting to placate. As will become evident,
no such browser stands a chance in hell of running this script, so the
only explanation is cargo cult programming (i.e. they saw this line in
another script and copied it without understanding the ramifications).
/**
* @class Ext
There are no classes in ECMAScript implementations.
* Ext core utilities and functions.
Everything old is new again.
* @singleton
It goes without saying that there are no singletons either.
*/
Ext.setup = function(config) {
if (Ext.isObject(config)) {
Oh dear. We'll get to that one shortly. Suffice to say that there is
no call for this (no pun intended).
if (config.addMetaTags !== false) {
var viewport = Ext.get(document.createElement('meta')),
app = Ext.get(document.createElement('meta')),
statusBar = Ext.get(document.createElement('meta')),
startupScreen =
Ext.get(document.createElement('link')),
appIcon = Ext.get(document.createElement('link'));
Okay. Five elements created.
viewport.set({
name: 'viewport',
content: 'width=device-width, user-scalable=no,
initial-scale=1.0, maximum-scale=1.0;'
});
No call for this either. As we'll see, the "set" function is another
botched attempt at setting attributes and/or properties.
if (config.fullscreen !== false) {
app.set({
name: 'apple-mobile-web-app-capable',
content: 'yes'
});
Ditto.
if (Ext.isString(config.statusBarStyle)) {
The isString function is also dubious.
statusBar.set({
name: 'apple-mobile-web-app-status-bar-style',
content: config.statusBarStyle
});
}
}
if (Ext.isString(config.tabletStartupScreen) &&
Ext.platform.isTablet) {
Ext.platform is populated largely by UA-based browser sniffing.
startupScreen.set({
rel: 'apple-touch-startup-image',
href: config.tabletStartupScreen
});
} else if (Ext.isString(config.phoneStartupScreen) &&
Ext.platform.isPhone) {
startupScreen.set({
rel: 'apple-touch-startup-image',
href: config.phoneStartupScreen
});
}
if (config.icon) {
config.phoneIcon = config.tabletIcon = config.icon;
}
This is a very odd design. Why have three properties? If they were
going to allow specifying separate icons for what they "detect" as
phones and tablets, then surely they shouldn't step on them without
looking.
var precomposed = (config.glossOnIcon == false) ? '-
precomposed' : '';
if (Ext.isString(config.tabletIcon) &&
Ext.platform.isTablet) {
Why didn't they just check config.icon?
appIcon.set({
rel: 'apple-touch-icon' + precomposed,
href: config.tabletIcon
});
} else if (Ext.isString(config.phoneIcon) &&
Ext.platform.isPhone) {
appIcon.set({
el: 'apple-touch-icon' + precomposed,
href: config.phoneIcon
});
}
var head = Ext.get(document.getElementsByTagName('head')
[0]);
Why pass the result to Ext.get? Is there some ill-advised host object
augmentation going on here?
head.appendChild(viewport);
if (app.getAttribute('name')) head.appendChild(app);
if (statusBar.getAttribute('name'))
head.appendChild(statusBar);
if (appIcon.getAttribute('href'))
head.appendChild(appIcon);
if (startupScreen.getAttribute('href'))
head.appendChild(startupScreen);
}
Nope. They used only standard DOM methods. Of course, there was no
need to call getAttribute at all. They could have just checked the
corresponding DOM properties; but more importantly, as seen above,
they are the ones who set (or didn't set) these attributes in the
first place. In other words, the logic takes the long way around,
unnecessarily involving one of the least trustworthy methods in the
history of browser scripting (getAttribute).
And why did they create all of those elements in advance when some or
all of them may not be appended at all? In short, the whole preceding
mess could be re-factored in five minutes to save several function and
host method calls, not to mention the creation of up to five elements.
When choosing a browser script, one of the first questions should be
who: wrote it and what is their relative level of proficiency? It's
not a "personal smear" to make a judgment call at this point. The
authors are obviously yet another batch of clueless neophytes (see
also jQuery, Prototype, Dojo, YUI, ExtJS, MooTools, etc.) Suffice to
say that leaning on a script written by such authors is going to lead
to problems. Readers with even the slightest experience in cross-
browser scripting can stamp this one "Avoid at all Costs" and move on
at this point (if they haven't already).
if (Ext.isArray(config.preloadImages)) {
Oops. There is no way to write a reliable "isArray" function in JS.
And as above, there is no reason to do anything more than a boolean
type conversion here (as long as the documentation indicates that this
property must be an array). Anything "truthy" that is not an array
will result in a silent failure as written, which is the least helpful
result (often described as "robustness" by library authors).
for (var i = config.preloadImages.length - 1; i >= 0; i--)
{
How about:-
for (var i = config.preloadImages.length; i--
{Sure that's a minor quibble, but it is yet another glimpse into the
authors' proficiency (or lack thereof). We are only a few dozen lines
in and virtually every line needs to be rewritten (which should take
*one* proficient JS developer about ten minutes).
(new Image()).src = config.preloadImages;
};
}
if (Ext.isFunction(config.onReady)) {
Ext.onReady(config.onReady, config.scope || window);
}
As we'll see, the isFunction function is yet another dubious entry.
And scope has *nothing* to do with the - this - despite the insistence
of seemingly every "major" library author. In this case, it's not
just a naming snafu as the ExtJS developers constantly refer to - this
- as "scope" in their documentation and support forums. And if they
don't understand why that is wrong, they haven't gotten past the first
page of the manual. I wonder what they call scope. Bananas?
Seriously, you have to wonder if these developers understand the
language they are using at all.
Ext.apply = function(object, config, defaults) {
// no "this" reference for friendly out of scope calls
There they go again.
if (defaults) {
What? No isObject call?
Ext.apply(object, defaults);
}
if (object && config && typeof config == 'object') {
for (var key in config) {
Oops. Unfiltered for-in loops are a very bad idea (at least for
scripts deployed on the Web). If this script is mashed up with
something that augments Object.prototype (e.g. older versions of the
unfortunately named Prototype library), all hell will break loose here
(in the deepest part of their core).
object[key] = config[key];
}
}
return object;
};
Calling this function "apply" was a pretty silly idea as well (because
of Function.prototype.apply). You've got to wonder if the authors are
that out of touch (no pun intended) or simply trying to confuse
beginners to keep them dependent on their dubious offerings. Both are
unsavory prospects.
Ext.apply(Ext, {
userAgent: navigator.userAgent.toLowerCase(),
Nellie bar the door. As has been known for a decade, referencing this
deception device cannot lead to anything good.
applyIf : function(object, config) {
var property, undefined;
There's no need to declare a local "undefined" identifier.
if (object) {
for (property in config) {
Another unfiltered for-in.
if (object[property] === undefined) {
Just use the typeof operator.
object[property] = config[property];
}
}
}
return object;
},
/**
* Repaints the whole page. This fixes frequently encountered
painting issues in mobile Safari.
*/
Their "fix" is nothing but a mystical incantation. Clearly their
script has had problems with the one platform they seek to support,
but rather than attempting to understand the cause of these problems,
they've resorted to nonsense code that appears to "fix" the problem in
whatever mobile devices they had handy to test with at the time of
writing.
repaint : function() {
var mask = Ext.getBody().createChild({
cls: 'x-mask x-mask-transparent'
});
Here they create and immediately discard a wrapper object for
document.body.
setTimeout(function() {
mask.remove();
}, 0);
ISTM that using 0 for the second argument to setTimeout is ill-advised
(as is the implied global).
},
/**
* Generates unique ids. If the element already has an id, it is
unchanged
* @param {Mixed} el (optional) The element to generate an id for
* @param {String} prefix (optional) Id prefix (defaults "ext-
gen")
* @return {String} The generated Id.
*/
id : function(el, prefix) {
return (el = Ext.getDom(el) || {}).id = el.id || (prefix ||
"ext-gen") + (++Ext.idSeed);
},
That's just plain ridiculous and falls under the category of "concise"
code that is all the rage these days. How about something like this:-
var id;
if (typeof el == 'string') { // ID passed, find it
el = document.getElementById(el);
}
if (el) { // Element exists
if (el.id) { // Has an ID
id = el.id;
} else { // Does not have an ID, assign
id = el.id = (prefix || "ext-gen") + (++Ext.idSeed);
}
}
return id; // undefined if element not found
It's not as if that will be any longer once white space and comments
are removed. Will be a hell of a lot easier to maintain and debug as
well (no phantom ID's generated).
[skipped tired old Ext "class" related functions]
urlEncode : function(o, pre){
var empty,
buf = [],
e = encodeURIComponent;
Ext.iterate(o, function(key, item){
Why not just use a for-in loop? If the (omitted) comments are to be
believed, then o is only allowed to be an Object object. The iterate
function makes calls to isObject, isEmpty and isIterable in a futile
attempt to support Object, Array and host objects with one function.
empty = Ext.isEmpty(item);
As we shall see, isEmpty is another unnecessary function and itself
calls isArray (of all things).
Ext.each(empty ? key : item, function(val){
buf.push('&', e(key), '=', (!Ext.isEmpty(val) && (val !
= key || !empty)) ? (Ext.isDate(val) ? Ext.encode(val).replace(/"/g,
'') : e(val)) : '');
});
});
Hmmm. Skipping ahead, the Ext.encode function is defined as:-
/**
* Shorthand for {@link Ext.util.JSON#encode}
* @param {Mixed} o The variable to encode
* @return {String} The JSON string
* @member Ext
* @method encode
*/
Ext.encode = Ext.util.JSON.encode;
And the "longhand" version is defined as:-
/**
* @class Ext.util.JSON
How is an object literal a class?
* Modified version of Douglas Crockford"s json.js that doesn"t
* mess with the Object prototype
* http://www.json.org/js.html
* @singleton
*/
Ext.util.JSON = {
encode : function(o) {
return JSON.stringify(o);
},
Now what does JSON have to do with urlencoding dates?
So far none of this is anything close to professional code and much of
it is positively senseless. Still no sign of HTML5 either.
On to decoding:-
/**
* Takes an encoded URL and and converts it to an object. Example:
* <pre><code>
Ext.urlDecode("foo=1&bar=2"); // returns {foo: "1", bar: "2"}
Ext.urlDecode("foo=1&bar=2&bar=3&bar=4", false); // returns {foo: "1",
bar: ["2", "3", "4"]}
</code></pre>
Of course, none of those are URL's.
* @param {String} string
* @param {Boolean} overwrite (optional) Items of the same name
will overwrite previous values instead of creating an an array
(Defaults to false).
* @return {Object} A literal with members
A what?!
*/
urlDecode : function(string, overwrite){
if(Ext.isEmpty(string)){
return {};
}
var obj = {},
pairs = string.split('&'),
d = decodeURIComponent,
name,
value;
Ext.each(pairs, function(pair) {
pair = pair.split('=');
name = d(pair[0]);
value = d(pair[1]);
obj[name] = overwrite || !obj[name] ? value :
[].concat(obj[name]).concat(value);
});
return obj;
},
Interesting. No call to Ext.decode. I guess JSON is only related to
encoding URL's.
/**
* Convert certain characters (&, <, >, and ') to their HTML
character equivalents for literal display in web pages.
* @param {String} value The string to encode
* @return {String} The encoded text
*/
htmlEncode : function(value){
return Ext.util.Format.htmlEncode(value);
},
Unnecessary bloat and another unneeded function call. Odd choices for
a mobile framework.
/**
* Appends content to the query string of a URL, handling logic
for whether to place
* a question mark or ampersand.
* @param {String} url The URL to append to.
* @param {String} s The content to append to the URL.
* @return (String) The resulting URL
*/
urlAppend : function(url, s){
if(!Ext.isEmpty(s)){
Another half-dozen unneeded function calls. This would be bad enough
in and of itself, but the functions in question are highly dubious as
well. Try this:-
if (s) {
return url + (url.indexOf('?') === -1 ? '?' : '&') + s;
No need for a strict comparison there. It's ham-fisted and makes
future maintainers (and reviewers) have to pause and wonder why they
did it.
}
return url;
},
/**
* Converts any iterable (numeric indices and a length property)
into a true array
There's no such thing as an "iterable" or a "true array".
* Don't use this on strings. IE doesn't support "abc"[0] which
this implementation depends on.
Oh, I they needn't worry about IE.
* For strings, use this instead: "abc".match(/./g) => [a,b,c];
That's a syntax error (so don't use it).
* @param {Iterable} the iterable object to be turned into a true
Array.
* @return (Array) array
*/
toArray : function(array, start, end){
return Array.prototype.slice.call(array, start || 0, end ||
array.length);
That will blow up in IE < 9.
},
/**
* Iterates an array calling the supplied function.
Not according to the next line.
* @param {Array/NodeList/Mixed} array The array to be iterated.
If this
There is no way to reliably differentiate between Array and host
objects (so don't design systems that hinge on making that work). And
I don't know what a "Mixed" is supposed to be.
each : function(array, fn, scope) {
That's not scope!
if (Ext.isEmpty(array, true)) {
return 0;
}
Whatever. They'd have been much better off copying
Array.prototype.forEach (then they could use that when available).
if (!Ext.isIterable(array) || Ext.isPrimitive(array)) {
array = [array];
}
Now there's a bizarre disjunction. If it is not "iterable" or it is a
primitive? You have to wonder what sort of "iterable" their code
would identify as a primitive. It's positively Dojo-esque (meaning
full of incomprehensible intertwined type-checking calls where none
are needed).
for (var i = 0, len = array.length; i < len; i++) {
if (fn.call(scope || array, array, i, array) ===
false) {
Why would they set - this - to array? That's going to lead to some
odd results.
return i;
};
}
return true;
},
iterate : function(obj, fn, scope){
if(Ext.isEmpty(obj)){
return;
}
if (Ext.isIterable(obj)) {
Ext.each(obj, fn, scope);
return;
}
else if(Ext.isObject(obj)) {
for (var prop in obj) {
if (obj.hasOwnProperty(prop)) {
Finally, a filter!
Inconsistent with the rest though. You've gotto wonder if they simply forgot to filter the others. More likely
this is simply a slap-dash patchwork of previous scripts (e.g. ExtJS,
JQTouch).
if (fn.call(scope || obj, prop, obj[prop], obj)
=== false) {
return;
};
}
}
}
},
The indisputedly unreliable (and wholly unneeded) type-checking
permeates everything. All hopes of cross-browser compatibility are
lost (for no other reason than the authors were/are too green to
design a system this complicated).
* <b>Note</b>: the dom node to be found actually needs to exist
(be rendered, etc)
* when this method is called to be successful.
* @param {Mixed} el
Apparently "Mixed" means "botched". In this case, the function is
expected to discriminate between native and host objects. Such
"overloading" strategies are highly ill-advised in JS, yet seemingly
everything in this script relies on them.
* @return HTMLElement
*/
getDom : function(el) {
if (!el || !document) {
return null;
}
Now when is document going to type-convert to false?
return el.dom ? el.dom : (typeof el == 'string' ?
document.getElementById(el) : el);
},
/**
* Returns the current document body as an {@link Ext.Element}.
* @return Ext.Element The document body
*/
getBody : function(){
return Ext.get(document.body || false);
},
More gobbledygook. It's as if every line is designed with madness in
mind. The first line of Ext.get is:-
if(!el){
return null;
}
....and it's not as if document.body will be missing in either of the
two browsers they aspire to support. Regardless, why not something
like this:-
var body = document.body;
return body ? Ext.get(body) : null;
Function calls are not free and a "mobile framework" needs to be as
efficient as possible due to the limited resources available to mobile
browsers. By the same token, property access costs and this thing
uses them constantly when they could easily be avoided.
/**
* Returns the current HTML document object as an {@link
Ext.Element}.
* @return Ext.Element The document
*/
getDoc : function(){
return Ext.get(document);
},
This is jQuery-itis. In other words, an API abstraction that makes no
sense. Why would they wrap a document object in an Ext.Element
object. After all, elements and documents are very different types of
DOM nodes. What do the element-specific methods do for documents?
And assuming there are any document-specific methods, what do they do
for elements? It's mind-boggling. The only rationalization I've
heard for such a bizarre design is that Web developers could be
confused by more than one wrapper type.
/**
* This is shorthand reference to {@link Ext.ComponentMgr#get}.
* Looks up an existing {@link Ext.Component Component} by {@link
Ext.Component#id id}
* @param {String} id The component {@link Ext.Component#id id}
* @return Ext.Component The Component, <tt>undefined</tt> if not
found, or <tt>null</tt> if a
* Class was found.
*/
getCmp : function(id){
return Ext.ComponentMgr.get(id);
},
Another ridiculous waste of time and space.
So far, everything that can go wrong has gone wrong. Literally. No
doubt, this is somehow my fault. In other words, if I hadn't pointed
out all of these problems then they wouldn't exist.
/**
* Returns the current orientation of the mobile device
* @return {String} Either 'portrait' or 'landscape'
*/
getOrientation: function() {
return window.innerHeight > window.innerWidth ? 'portrait' :
'landscape';
},
Highly dubious and virtually never needed. In the same way that older
libraries attempt to replace rock-solid mechanisms like IE's
conditional comments with browser sniffing, this attempts to can CSS3
media queries.
http://www.w3.org/TR/css3-mediaqueries/#orientation
Of course, you can't sell what you can't can.
/**
* <p>Removes this element from the document, removes all DOM
event listeners, and deletes the cache reference.
* All DOM event listeners are removed from this element. If
{@link Ext#enableNestedListenerRemoval} is
* <code>true</code>, then DOM event listeners are also removed
from all child nodes. The body node
* will be ignored if passed in.</p>
* @param {HTMLElement} node The node to remove
*/
removeNode : function(n){
if (n && n.parentNode && n.tagName != 'BODY') {
Checking for the body is ludicrous. If the calling application fouls
up and passes the body, a silent failure will only obscure the
mistake, making it harder for the developers to track it down. Same
for checking the parentNode property.
Ext.EventManager.removeAll(n);
Almost certainly unneeded. If their EventManager thing actually
creates circular references then they should fix that, not dance
around the problem with this outdated Crockfordian strategy. And
regardless, this script won't work with IE (the browser with the
circular reference problem) anyway.
n.parentNode.removeChild(n);
delete Ext.cache[n.id];
}
},
/**
* Attempts to destroy any objects passed to it by removing all
event listeners, removing them from the
* DOM (if applicable) and calling their destroy functions (if
available). This method is primarily
* intended for arguments of type {@link Ext.Element} and {@link
Ext.Component}, but any subclass of
There are no classes in JS, so there can be no subclasses.
* {@link Ext.util.Observable} can be passed in. Any number of
elements and/or components can be
* passed into this function in a single call as separate
arguments.
* @param {Mixed} arg1 An {@link Ext.Element}, {@link
Ext.Component}, or an Array of either of these to destroy
Again, you cannot reliably tell an Object from an Array object.
Designing such systems in JS is like deliberately walking into a wall
(over and over in the case of this script).
* @param {Mixed} arg2 (optional)
* @param {Mixed} etc... (optional)
*/
destroy : function() {
var ln = arguments.length,
i, arg;
for (i = 0; i < ln; i++) {
arg = arguments;
if (arg) {
if (Ext.isArray(arg)) {
this.destroy.apply(this, arg);
}
else if (Ext.isFunction(arg.destroy)) {
arg.destroy();
}
else if (arg.dom) {
arg.remove();
}
}
}
},
isIterable : function(v){
//check for array or arguments
if(Ext.isArray(v) || v.callee){
return true;
}
The isArray function is unreliable and the presence of a "callee"
property does not indicate anything close to what they are trying to
determine (see their previous definition of an "iterable"). And
you've got to wonder what sort of strange design would require passing
the arguments object to a function like this. I mean, the only way a
variable could reference an arguments objects is by explicitly setting
it to reference an arguments object.
//check for node list type
if(/NodeList|
HTMLCollection/.test(Object.prototype.toString.call(v))){
return true;
}
Not only is the RegExp botched, but host objects are allowed to return
*anything* in response to a toString call. For example, this will
fail in many versions of IE (and presumably at least some of its
mobile derivations). Of course, this script will fall flat on its
face in IE anyway, for no reason other than dubious design decisions.
//NodeList has an item and length property
//IXMLDOMNodeList has nextNode method, needs to be checked
first.
Suffice to say that an application that needs to pass an XML nodelist
to this function is doomed from the start. Likely that includes the
framework itself.
return ((typeof v.nextNode != 'undefined' || v.item) &&
Ext.isNumber(v.length));
},
Ext.isNumber is another one of the botched (and unneeded) type-
checking functions.
So obviously, the "isIterable" function is an untenable (and unneeded)
design. And each time the authors hit upon a host object that
returned the "wrong" result they added another set of checks to make
it "right", instead of realizing they were pissing in the wind from
the start. Again, very Dojo-esque.
/**
* Utility method for validating that a value is numeric,
returning the specified default value if it is not.
* @param {Mixed} value Should be a number, but any type will be
handled appropriately
* @param {Number} defaultValue The value to return if the
original value is non-numeric
* @return {Number} Value, if numeric, else defaultValue
*/
num : function(v, defaultValue){
v = Number(Ext.isEmpty(v) || Ext.isArray(v) || typeof v ==
'boolean' || (typeof v == 'string' && v.trim().length == 0) ? NaN :
v);
return isNaN(v) ? defaultValue : v;
},
Another faulty design, rendered in typically bizarre and unreliable
fashion (nothing that calls isArray can be considered reliable). And
you really must wonder what sort of application would need such a
function.
/**
* <p>Returns true if the passed value is empty.</p>
* <p>The value is deemed to be empty if it is<div class="mdetail-
params"><ul>
* <li>null</li>
* <li>undefined</li>
* <li>an empty array</li>
* <li>a zero length string (Unless the <tt>allowBlank</tt>
parameter is <tt>true</tt>)</li>
* </ul></div>
* @param {Mixed} value The value to test
* @param {Boolean} allowBlank (optional) true to allow empty
strings (defaults to false)
* @return {Boolean}
*/
isEmpty : function(v, allowBlank) {
return v == null || ((Ext.isArray(v) && !v.length)) || (!
allowBlank ? v === '' : false);
},
Another call to isArray, which means the previous functions results in
two calls to that unreliable function. There appears to be no real
design work here, just a bunch of ill-advised functions tangled up in
haphazard fashion. I can hear the chorus of "show me where it fails"
now.
And again, read the description and wonder what sort of strange
application would need such a function.
/**
* Returns true if the passed value is a JavaScript array,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isArray : function(v) {
return Object.prototype.toString.apply(v) === '[object
Array]';
},
There it is. The "Miller Device" has been proven unreliable.
/**
* Returns true if the passed object is a JavaScript date object,
otherwise false.
* @param {Object} object The object to test
* @return {Boolean}
*/
isDate : function(v) {
return Object.prototype.toString.apply(v) === '[object Date]';
},
Same.
/**
* Returns true if the passed value is a JavaScript Object,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isObject : function(v) {
return !!v && Object.prototype.toString.call(v) === '[object
Object]';
},
The left part of the conjunction is clearly unneeded. And if they
would just stop to think about their design, they'd realize that a
typeof test would do just as well if they disallowed host objects as
arguments to this function (which they should do anyway).
/**
* Returns true if the passed value is a JavaScript 'primitive', a
string, number or boolean.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isPrimitive : function(v) {
return Ext.isString(v) || Ext.isNumber(v) || Ext.isBoolean(v);
},
This review could really be compressed to a string of "unneeded and
botched" comments (a good ways through the core of this thing nothing
is close to competently designed or implemented). Trying to learn
anything about browser scripting from the code and/or documentation of
scripts like this is hopeless as the authors mangle virtually every
aspect of the language (e.g. scope, primitives, literals, etc.) Yet
somehow many developers have come to the conclusion that they *must*
abdicate all responsibility for browser scripting to "expert" efforts
like this one.
/**
* Returns true if the passed value is a JavaScript Function,
otherwise false.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isFunction : function(v) {
return Object.prototype.toString.apply(v) === '[object
Function]';
},
Unneeded and botched. The ambiguities that led to the advent of the
(unreliable) "Miller Device" do not apply to Function objects (i.e.
just use typeof). But I suppose the "designers" wanted to allow for
discrimination between RegExp, Function and host objects, despite the
fact that there is no practical application for such testing.
Ironically, defenders of such scripts often pepper their retorts with
references to the "real world".
/**
* Returns true if the passed value is a number. Returns false for
non-finite numbers.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isNumber : function(v) {
return Object.prototype.toString.apply(v) === '[object
Number]' && isFinite(v);
},
Unneeded and botched. Recall that isPrimitive calls this, yet it is
clearly "designed" to allow Number objects to slip through.
/**
* Returns true if the passed value is a string.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isString : function(v) {
return Object.prototype.toString.apply(v) === '[object
String]';
},
Unneeded and botched (see previous).
/**util
* Returns true if the passed value is a boolean.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isBoolean : function(v) {
return Object.prototype.toString.apply(v) === '[object
Boolean]';
},
Clearly we are dealing with a cargo cult here, copying and pasting
patterns of code without the slightest understanding of what it does.
This one reduces to:-
isBoolean : function(v) {
return typeof v == 'boolean';
},
....which is clearly a waste of a function call (why wouldn't you just
use the typeof operator in the first place?)
/**
* Returns true if the passed value is an HTMLElement
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isElement : function(v) {
return !!v && v.tagName;
},
Unneeded and botched.
/**
* Returns true if the passed value is not undefined.
* @param {Mixed} value The value to test
* @return {Boolean}
*/
isDefined : function(v){
return typeof v !== 'undefined';
},
Unneeded function with an unneeded strict comparison.
/**
* URL to a blank file used by Ext when in secure mode for iframe src
and onReady src to prevent
* the IE insecure content warning (<tt>'about:blank'</tt>, except for
IE in secure mode, which is <tt>'javascript:""'</tt>).
* @type String
*/
Ext.SSL_SECURE_URL = Ext.isSecure && 'about:blank';
Huh? This is isSecure:-
isSecure : /^https/i.test(window.location.protocol),
....so if the document is not served with the https protocol, the above
will be false (which is obviously not a string).
Ext.ns = Ext.namespace;
Why? To save themselves keystrokes? Or perhaps this is a misguided
attempt to shorten the download. Either way it makes no sense.
I'm still looking for the first bit that indicates some semblance of
sense. Though I know there are those who will dismiss this review in
light of the really "cool" Solitaire demo. In other words, regardless
of how bad the code is, if any sort of application can be demonstrated
to "work" in at least two browsers, then arguments about the quality
of the code can be dismissed out of hand (commonly phrased as "we
write code for users!").
Ext.ns(
'Ext.util',
'Ext.data',
'Ext.list',
'Ext.form',
'Ext.menu',
'Ext.state',
'Ext.layout',
'Ext.app',
'Ext.ux',
'Ext.plugins',
'Ext.direct'
);
Another waste of time and space, though I am sure some misguided
programmers would consider this more "advanced" than simple assigning
properties to the Ext object.
I skipped the "namespace" function; here it is:-
/**
* Creates namespaces to be used for scoping variables and classes
so that they are not global.
Scoping?! This (repeated) criticism is not being petty. I run into
programmers all the time who use the term "scope" to refer to all
sorts of things that have nothing to do with scope. I have to wonder
what word they would use if they actually wanted to discuss scope
(it's not as if scope is an arcane subject in the context of JS).
Scripts and comments like this are to blame.
* Specifying the last node of a namespace implicitly creates all
other nodes. Usage:
* <pre><code>
Ext.namespace('Company', 'Company.data');
Ext.namespace('Company.data'); // equivalent and preferable to above
syntax
Company.Widget = function() { ... }
Company.data.CustomStore = function(config) { ... }
</code></pre>
* @param {String} namespace1
* @param {String} namespace2
* @param {String} etc
* @return {Object} The namespace object. (If multiple arguments
are passed, this will be the last namespace created)
* @method namespace
*/
namespace : function() {
var ln = arguments.length,
i, value, split, x, xln;
for (i = 0; i < ln; i++) {
value = arguments;
parts = value.split(".");
object = window[parts[0]] = Object(window[parts[0]]);
As mentioned, using the window object in lieu of a reference to the
global object is a common rookie mistake.
And why are they calling Object? I suppose it is in the name of
"robustness" again, obscuring rather than exposing problems and making
the debugging of applications more difficult.
For example, assume that the global "Ext" variable unexpectedly has
been set to the primitive value of 5. Passing that to Object will
result in a Number object, not an Object object (clearly not what was
desired). The application developer should be aware that something
had gone horribly wrong prior to the conversion, so forcing the issue
is counter-productive.
for (x = 1, xln = parts.length; x < xln; x++) {
object = object[parts[x]] = Object(object[parts[x]]);
More of the same.
}
}
return object;
},
[Skip the usual ExtJS function wrapper suspects]
/**
* @class String
* These functions are available on every String object.
*/
Ext.applyIf(String.prototype, {
Good night.
Never did get to the HTML5 bits. Having reviewed the script in its
entirety for a client, I know they are few and far between. There's a
dozen line function to create AUDIO elements (complete with browser
sniffing and lacking any sort of feature detection), and another that
does the same for VIDEO elements. I don't know if localStorage is
part of the "official" HTML5 effort, but they've got a wrapper for
that. Oh and another dozen lines dealing with geolocation (also fuzzy
on whether that is part of HTML5). IIRC, that's about it.
As for the "touch" part. They've attempted to smooth out the
differences in the ontouch* event handling (between the two browsers
they aspire to support) using more browser sniffing. Also IIRC, they
attempt to synthesize the ongesture* events based on touches. Like
the rest of it, it's a tangled up house of cards.
Pretty lousy name too. Sencha doesn't exactly roll off the tongue,
does it?