RFC: Building the Perfect Tabbed Pane (an tutorial article)

P

Peter Michaux

Now I need to fire up a bunch of browsers and accumulate examples of
all these weird cases.

Safari 3 on OS X
----------------

typeof document['all'] // 'undefined'

document['all'] // [object HTMLCollection]'


That's a really nice one!

Somewhere I wrote I was worried about exactly this in December. The
ECMAScript spec says in section 11.4.3 that a host object can return
anything for the "typeof" operator. Unfortunately, anything includes
"undefined".

This is a false negative for the isHostCollection(this.document,
'all') and that is better than a false positive. The false negative in
Safari for document.all is not a monumental loss but so much for any
amount of "proof" that the test functions work universally.

By the way, this is the very first test I tried on a few browsers and
unfortunately already there is a problem.

Peter
 
P

Peter Michaux

On Feb 14, 6:51 pm, Peter Michaux <[email protected]> wrote:
2. News (NNTP) links. Type converting or assigning the href property
blows up IE7.


Type converting to boolean and string both work for me

var el = document.getElementById('newsLink');
if (el.href) {alert('ok');}

var el = document.getElementById('newsLink');
alert(el.href);

-------------


Assigning works fine for me.

<p><a id="newsLink" href="
var el = document.getElementById('newsLink');
el.href = '
When I click the link Outlook wants me to join comp.lang.javascript

This is a Win XP with a single IE7 install. No other IE's installed.

Peter
 
D

David Mark

Now I need to fire up a bunch of browsers and accumulate examples of
all these weird cases.

Safari 3 on OS X
----------------

typeof document['all'] // 'undefined'

document['all']        // [object HTMLCollection]'

That's a really nice one!

Yes. Well, then Mac Safari 3 will be treated as if document.all did
not exist. Something tells me that is how the Safari developers
wanted it. It may be included chiefly to defeat object inferences
(which typically use boolean type conversion.)
Somewhere I wrote I was worried about exactly this in December. The
ECMAScript spec says in section 11.4.3 that a host object can return
anything for the "typeof" operator. Unfortunately, anything includes
"undefined".

True enough. But there's not much we can do about that.
This is a false negative for the isHostCollection(this.document,
'all') and that is better than a false positive. The false negative in
Safari for document.all is not a monumental loss but so much for any
amount of "proof" that the test functions work universally.

I never meant to imply that they worked universally. I just meant
that the functions prevented IE's strange exceptions and allowed for
collections that implement an internal [[Call]] method. There's no
way to know if there are agents out there that return "mickeymouse" or
"fooledyou" or whatever from typeof operations. If developers of
agents want their products to pass feature tests for rich content,
they have to provide some semblance of compatibility, which will
hopefully prevent them from venturing too far off the beaten path.
By the way, this is the very first test I tried on a few browsers and
unfortunately already there is a problem.

It depends on how you look at it. As far as I am concerned, a
collection object with an undefined type is best treated as a non-
entity. I suspect you won't see similar results for document.images,
document.forms, etc. in that browser.
 
P

Peter Michaux

I wouldn't! I'd have to add another test to isHostMethod.

I meant it as a joke but now I'm taking it seriously, especially after
reviewing the spec and the odd ball examples we've examined. If they
add that we should not have to change our code because our code is too
strict. I'm going to bumble a bit in this explanation but I think it
is right...or at least not wrong.

The basic use of the current three functions are to determine if a
host object has a particular property or not. We are testing
existance. It seems that typeof has an implicit try-catch and so that
is why we are using instead of to-boolean type converting tests. Also
the type converting tests don't provide information in IE when they
error even if the type conversion test was wrapped in a real try-
catch. Using typeof may be the only option for testing.

The typeof a host object property can be *anything*. Those are the
rules. What we are doing with the three functions is too strict and
not looking to the future where more browsers may report more values
for typeof. We aren't even looking to every current browser that may
already return other typeof values.

Feature testing is a technique that has the ultimate goal that if the
code goes to a browser we have never seen, if the features are there,
the code runs.

When we are using isHostMethod we are checking if the type is
"unknown", "function", or "object" (except for null). That is too
strict. If a browser reports "BillGates" as the type then we still
know the object property exists. If it did not exist then typeof would
return "undefined". Only because of a bug in the language we have to
test if a property is null when "object" is reported.

If we are expecting a callable property and it's typeof is not
"undefined" then we should consider it to exist and be callable. This
is presumptuous but not crazy.

What we are doing with the current isHostMethod is equally
presumputous because we are say that just because the browser reports
"unknown" we assume the property is callable because we expected it to
be callable. Yes it is based on experience but the idea it is no
better than what I'm suggesting.

Because of all this, I think the following is sufficient instead of
all three

var isHostProp = function(o, p) {
var t = typeof o[p];
return t != 'undefined' &&
(t != 'object' ||
o[p]);
};

This allows for any legitimate host object that complies with the spec
to pass. It avoid the IE [[Get]] error problem. It allows the
collections "function" objects pass. It takes care of the typeof null
bug in the language.

What the three functions do is reduce the number of things that will
pass the above code based on experience of what popular hosts return
for "typeof". However by limiting based on experience, compliant
objects can fail. These are false negatives which is ok but with what
benefit?

If we are basing design on experience, are their examples where
isHostObject, isHostCollection, isHostMethod are better than
isHostProp in practice?

Peter
 
P

Peter Michaux

var isHostProp = function(o, p) {
var t = typeof o[p];
return t != 'undefined' &&
(t != 'object' ||
o[p]);
};

The intention of the above is to find a replacement for a type
converting test which is the most natural test

if (doc.all) {
// use document.all
}

becomes

if (isHostProp(doc, 'all')) {
// use document.all
}

But we know that type converting tests aren't useful in a practical
sense given the behavior of IE and the chance that Microsoft may
change any of their callable host objects to be ActiveXObjects at any
time.

Both the type converting test and the isHostProp test give false
positives if document.all === true but is that a practical concern
that exists in some browser?

Suppose we use isHostCollection

if (isHostCollection(doc, 'all')) {
// use document.all
}

We still get false positives if document.all is some dummy function,
or defective object. We get less false positives but we start getting
false negatives for standard compliant objects. Are there real world
example where this price of false negatives is worth paying?

I can imagine that the following probably doesn't have any real world
problems. It would still have false negative for document.all on
Safari because it seems the WebKit devs want it that way.

var isHostProp2 = function(o, p) {
return typeof o[p] != 'undefined';
};

The following all work

isHostProp2(doc, 'all')
isHostProp2(doc, 'createElement')
isHostProp2(el, 'childNodes')


In which browsers do isHostProp or isHostProp2 or give false positives
for some property that isHostMethod/isHostObject/isHostCollection do
not? Which host object property out there is <code>null</code> in some
browser?

What I'm starting to dislike about isHostMethod, for example, is that
somehow there is the assumption that there is one way to test that any
property name is a function: check for three different typeof strings.
It would be very easy to design two new browsers that make this kind
of test fail.

-----------------------------

I'm starting to get a better big picture concept of how all this
feature testing business fits together.

General Goal: Feature testing is to avoid runtime errors. (Not
necessarily just JavaScript runtime errors but errors like rendering
in a browser that doesn't support enough CSS to make a widget "work"
correctly, for example.)

There are various kinds of inference and (almost) no matter which type
of test we use we will be at some level of inference.

1) navigator.userAgent inference (sniffing)

Since browsers can and do lie in navigator.userAgent looking at
substrings of this property is pointless. Very likely to give false
positives. This is the worst case of unrelated existence inference.

2) unrelated existence inference (sniffing)

If one object property exists, then infer that a different object
property will work. Checking for the existence of document.all or
ActiveXObject and then assuming the IE event model is available. Very
likely to give false positives.

3) related existence inference (sniffing)

Checking for the element.addEventListener and then
event.preventDefault is available. Less likely to give false positives
than unrelated existence inference but still sniffing. Gives a false
positive in Safari.

4) exemplar existence inference

Check that a property exists on an exemplary object. If it exists then
infer that it will work. This is the most common form of feature
testing. It should be sufficient. Are there examples where it is not?
Possible false positives.

5) specific existence inference

This checks for the existence of a property on a particular object but
doesn't assume another similar object will also have that property. If
the feature exists on a particular object then assume it works on that
object. For example, if we test for the existence of
document.documentElement.appendChild, we would not assume any other
DOM element has appendChild. Possible false positives. Somewhere
between existence inference and specific existence inference is why we
check both document.getElementsByTagName and
element.getElementsByTagName.

----

In addition to existence inference, you can analyze the object in a
variety of ways to insure the object satisfies your expectations about
how it should "look".

6) exemplar typeof inference

Check that a property exists and if that its typeof value is one of
the known values for a particular set of browsers then infer the
feature will work. Less likely to produce false positives than simple
existence testing. A host object can return any value when used in a
typeof operation so this can produce false negatives for ECMAScript
compliant host objects that have typeof values you don't know about.
Using typeof inference in addition to existence inference is more
conservative but perhaps without any known benefit over existence
inference alone.

7) specific typeof inference

similar

8) exemplar non-bad-value inference

Check that a host object property exists is not in a particular set of
known bad values, then infer the feature will work. For example, a
host object property is <code>null</code> it is defined and exists but
that value may be no good to you. Reduces the false positives of
existence inference alone.

9) specific non-bad-value inference

similar

----

10) exemplar object, exemplar use inference

Check that a property exists on an exemplar object and then try using
it. If it works then infer it will work again on similar objects.
Unlikely to give a false positive.

11) specific object, exemplar use inference

similar

12) specific object, specific use *testing*

Check that a property exists on a specific object, use it, and check
that it worked as expected after each use. Does not give false
positives. This is computationally expensive and likely not practical
in many situations. If the feature executes but produces a poor result
there is no guarantee you can reverse the operation. The reversal may
fail.

----------

Determining the right level of inference is not an objective decision.
File download size, computation expense and chance of false negatives
trade-off against more code that is less likely to runtime error.

Peter
 
P

Peter Michaux

I'm starting to get a better big picture concept of how all this
feature testing business fits together.

There is also

Syntax Inference (sniffing)

Using syntax when it is not required where it is used, so that a
particular browser will syntax error. For example, using === where ==
is sufficient just because somewhere else in the script the browser
needs to support document.getElementById and IE4 doesn't have this
feature. This if very likely to produce false positives.

Peter
 
P

Peter Michaux

There is also

Related Execution Inference (sniffing)

If the code is executing infer that a language features exist and
works. This seems to be acceptable for old language features (e.g.
String.prototype.substring which was in JavaScript 1.0, JScript 1.0
and ECMAScript v1). The tradeoff for more robust inference tests is
considered unjustified.

Unrelated Execution Inference (sniffing)

If the code is executing infer that a host features exist and works.
For example, many programmers assume that if JavaScript is executing
then the necessary CSS support is also available.


Language Version Inference (sniffing)

The script type attributes with its version number will start to play
a role in the future when ES4 is released. This happened a long time
ago with the script language attribute and JavaScript versioning. That
was before my time however.


Peter
 
D

David Mark

I meant it as a joke but now I'm taking it seriously, especially after
reviewing the spec and the odd ball examples we've examined. If they

Other than Mac Safari's seemingly intentional goof related to
document.all, what else is giving unexpected results?
add that we should not have to change our code because our code is too
strict. I'm going to bumble a bit in this explanation but I think it
is right...or at least not wrong.

The basic use of the current three functions are to determine if a
host object has a particular property or not. We are testing
existance. It seems that typeof has an implicit try-catch and so that
is why we are using instead of to-boolean type converting tests. Also
the type converting tests don't provide information in IE when they
error even if the type conversion test was wrapped in a real try-
catch. Using typeof may be the only option for testing.

I am sure that it is.
The typeof a host object property can be *anything*. Those are the
rules. What we are doing with the three functions is too strict and
not looking to the future where more browsers may report more values
for typeof. We aren't even looking to every current browser that may
already return other typeof values.

If the browser developers want to (deliberately or not) exclude
certain features from detection (e.g. by returning "fooledyou" for a
typeof operation), then that is their privilege. The only case I
think needs a workaround (as provided in isHostMethod) is the
"unknown" type in IE, as affected properties throw exceptions for some
operations.
Feature testing is a technique that has the ultimate goal that if the
code goes to a browser we have never seen, if the features are there,
the code runs.

In a perfect world, yes.
When we are using isHostMethod we are checking if the type is
"unknown", "function", or "object" (except for null). That is too
strict. If a browser reports "BillGates" as the type then we still

I wouldn't trust any object that reports such a type. Better to treat
it as a non-entity IMO.
know the object property exists. If it did not exist then typeof would
return "undefined". Only because of a bug in the language we have to
test if a property is null when "object" is reported.

If we are expecting a callable property and it's typeof is not
"undefined" then we should consider it to exist and be callable. This
is presumptuous but not crazy.

I disagree. If, for instance, it reports "boolean" or "number", I
wouldn't expect it to be callable.
What we are doing with the current isHostMethod is equally
presumputous because we are say that just because the browser reports
"unknown" we assume the property is callable because we expected it to

Not exactly. If it reports "unknown" and we are *expecting* a
callable property, then we can make the presumption. That's why you
should never do something like:

if (isHostMethod(el, 'href')) {
...
}

As we have seen, href properties of news links (and possibly others)
can be of type "unknown." However, it would be folly to expect them
to be callable and therefore isHostMethod is the wrong function to
test them with. I didn't bother to make a function to test non-
callable "unknown" properties as they are unusable for anything (can't
assign them, can't type convert them, etc.) See the getAttribute
wrapper, which returns the string "[unknown]" if the fetched property
is of an unknown type (only done in the IE workaround branch.)
be callable. Yes it is based on experience but the idea it is no
better than what I'm suggesting.

Because of all this, I think the following is sufficient instead of
all three

var isHostProp = function(o, p) {
  var t = typeof o[p];
  return t != 'undefined' &&
         (t != 'object' ||
          o[p]);

};

I don't like that. There is no way that I would allow my code to call
a property that returns "boolean", "number", etc. It is possible that
I might have to edit one function in the future if browser developers
introduce a "callable" or "iamafunction" type or whatever, but I am
fine with that (and don't see it happening anyway.)
This allows for any legitimate host object that complies with the spec
to pass. It avoid the IE [[Get]] error problem. It allows the
collections "function" objects pass. It takes care of the typeof null
bug in the language.

It is too generalized. For instance:

if (isHostProp(doc, 'images')) {
docimages = function(docNode) {
return (docNode || doc).images;
};
}

That allows for the images property to be an unknown type, which would
cause the docimages function to throw an exception every time (at
least that is the observed pattern.)
What the three functions do is reduce the number of things that will
pass the above code based on experience of what popular hosts return
for "typeof". However by limiting based on experience, compliant
objects can fail. These are false negatives which is ok but with what
benefit?

But what compliant objects? What browser developer would deliberately
exclude their host objects from such commonly used feature detection
logic? Other than the "unknown" workaround, the logic in the three
functions isn't ground-breaking.
If we are basing design on experience, are their examples where
isHostObject, isHostCollection, isHostMethod are better than
isHostProp in practice?

Similar to the above example:

if (isHostProp(this, 'document')) { ... }

This would allow a global "document" variable of any defined type to
trick the calling code into thinking it is running in a browser.

I firmly believe that each of the three functions has its place.
Using each as appropriate makes the intentions of gateway code clear.
Future maintenance may be needed for any of the three, but I wouldn't
assume that such maintenance would be avoidable if all three are
lumped together (in fact, it may require them to be broken up, which
would require rewriting all of the calling code.)
 
D

David Mark

var isHostProp = function(o, p) {
  var t = typeof o[p];
  return t != 'undefined' &&
         (t != 'object' ||
          o[p]);
};

The intention of the above is to find a replacement for a type
converting test which is the most natural test

if (doc.all) {
  // use document.all

}

becomes

if (isHostProp(doc, 'all')) {
  // use document.all

}

If you think about it, the two are basically equivalent (and too
general IMO), except for "unknown" types and Mac Safari's
implementation of document.all.
But we know that type converting tests aren't useful in a practical
sense given the behavior of IE and the chance that Microsoft may
change any of their callable host objects to be ActiveXObjects at any
time.

It has been my assumption that ActiveX implementations are responsible
for the "unknown" types. However, I don't believe that the callable
host object properties (or non-callable examples like href's of news
links) are ActiveX objects. It seems to me that "unknown" indicates
that they are properties of an ActiveX object (i.e. a news link
element is an ActiveX object and therefore its properties are of
unknown types.) The ActiveX object itself returns "object" for typeof
operations and does not throw an exception when assigned or type
converted (unlike its "unknown" properties.) Only MS knows for sure
if ActiveX is even involved in this mess, but it seems like a good
assumption in that "IUnknown" is the base interface for COM objects
(which were renamed "ActiveX" by the marketing department when IE3
came out, presumably because it sounded cooler.)
Both the type converting test and the isHostProp test give false
positives if document.all === true but is that a practical concern
that exists in some browser?

Just as with the "BillGates" example, it is hard to qualify concerns
about a hypothetical situation. I think the above example is more
likely though. For instance, an off-brand browser developer might do
such a thing to defeat object inferences (which seems to be what the
Mac Safari developers did with document.all.)

Bill, if you are out there, can you chime in on this? Will MS be
honoring your departure with a namesake host object type?
Suppose we use isHostCollection

if (isHostCollection(doc, 'all')) {
  // use document.all

}

We still get false positives if document.all is some dummy function,

There's nothing we can do about that in the detection phase. However,
the method can be tested further if needed (i.e. does it return an
object with a length property and does the first item have a tagName
property?)
or defective object.  We get less false positives but we start getting
false negatives for standard compliant objects. Are there real world
example where this price of false negatives is worth paying?

I can't think of a known false negative that would be avoided by
combining the three functions. As for the hypothetical future
"mickeymouse" types, I would rather they return a negative result, at
least until their behavior could be studied in the same way the
"unknown" types were.
I can imagine that the following probably doesn't have any real world
problems. It would still have false negative for document.all on
Safari because it seems the WebKit devs want it that way.

var isHostProp2 = function(o, p) {
  return typeof o[p] != 'undefined';

};

That allows for null. Without checking for "unknown", there is no way
to know if a further type conversion test to exclude it will throw an
exception in IE.
The following all work

isHostProp2(doc, 'all')
isHostProp2(doc, 'createElement')
isHostProp2(el, 'childNodes')

It also allows for:

if (isHostProp2(el, 'offsetParent')) {
alert(el.offsetParent.tagName);
}

This example will blow up for fixed position elements in Opera
(offsetParent === null.)

This one will throw an exception for elements in IE that are not part
of a document:

if (isHostProp2(el, 'offsetParent')) {
alert(el.offsetParent);
}

Which of the three tests to use depends on how the tested property is
to be used.
In which browsers do isHostProp or isHostProp2 or give false positives

I covered isHostProp2 above. The issue with isHostProp is not
necessarily false positives, but inappropriate positives (results that
are not meaningful in the context of a specific test.)
for some property that isHostMethod/isHostObject/isHostCollection do
not? Which host object property out there is <code>null</code> in some
browser?

There's no telling, but I think it is a more likely scenario than IE8
returning "BillGates" for typeof document.getElementById or some
undiscovered agent returning "xyz" for the same.
What I'm starting to dislike about isHostMethod, for example, is that
somehow there is the assumption that there is one way to test that any
property name is a function: check for three different typeof strings.

It is only used to verify properties that are expected to be methods
(e.g. gEBI.)
It would be very easy to design two new browsers that make this kind
of test fail.

Sure, just return "xyz" for typeof anything. But what browser
developer would do such a thing? Perhaps one that doesn't want its
host objects to be used?
-----------------------------

I'm starting to get a better big picture concept of how all this
feature testing business fits together.

General Goal: Feature testing is to avoid runtime errors. (Not

It is important to make a distinction between feature detection (the
purpose of the three functions at hand) and feature testing, which is
beyond their scope.

[snip]

I read through the rest of it. It is interesting and seems a useful
discussion for another article. However, for the purposes of the CWR
project (and this example widget), I think we need to settle the
feature detection layer sooner rather than later (we could debate this
subject forever.)

I really thought we were set with the slightly renamed trio as
proposed yesterday. That's what I am going with for my library (for
better or worse.)
 
P

Peter Michaux

I can't think of a known false negative that would be avoided by
combining the three functions. As for the hypothetical future
"mickeymouse" types, I would rather they return a negative result, at
least until their behavior could be studied in the same way the
"unknown" types were.

I think that is a very strong argument for the three.


It also allows for:

if (isHostProp2(el, 'offsetParent')) {
alert(el.offsetParent.tagName);

}

This example will blow up for fixed position elements in Opera
(offsetParent === null.)

This one will throw an exception for elements in IE that are not part
of a document:

if (isHostProp2(el, 'offsetParent')) {
alert(el.offsetParent);

}

My thinking was a little overzealous with regard to null values in
particular.



I read through the rest of it. It is interesting and seems a useful
discussion for another article.

It is one now :)

<URL: http://peter.michaux.ca/article/7146>

I'd like to put a link to your code if you have one publicly
available.
I really thought we were set with the slightly renamed trio as
proposed yesterday. That's what I am going with for my library (for
better or worse.)

I agree with that set. I just had some last minute second thoughts.

Peter
 
P

Peter Michaux

Other than Mac Safari's seemingly intentional goof related to
document.all, what else is giving unexpected results?

The ActiveX objects are weird.

[snip]
But what compliant objects?

None that I know of.

What browser developer would deliberately
exclude their host objects from such commonly used feature detection
logic?

Probably none.

[snip]
I firmly believe that each of the three functions has its place.
Using each as appropriate makes the intentions of gateway code clear.
Future maintenance may be needed for any of the three, but I wouldn't
assume that such maintenance would be avoidable if all three are
lumped together (in fact, it may require them to be broken up, which
would require rewriting all of the calling code.)

It is true.

Peter
 
D

David Mark

I think that is a very strong argument for the three.

I am glad you agree. I agree that none of this is guaranteed to be
100% future-proof or to support every browser ever released (a claim
that would be impossible to prove.)
My thinking was a little overzealous with regard to null values in
particular.


It is one now :)

<URL:http://peter.michaux.ca/article/7146>

I'll check it out.
I'd like to put a link to your code if you have one publicly
available.

You mean "My Library?" I don't want the public in there yet. The
version that is online is a very stale Alpha. When I get a free
moment (been busy with other projects for the last few weeks), I will
update it and post a link here.
I agree with that set. I just had some last minute second thoughts.

You brought up some valid points that needed to be addressed. The
specs vs. reality issue is a tricky one. In the end, I think that the
reality-based solution wins out (for now.)
 
D

David Mark

The ActiveX objects are weird.

Indeed. Thanks MS!
[snip]
But what compliant objects?

None that I know of.

Hopefully none will turn up. If one does, scripts should still
degrade gracefully, as long as gateway code calls the appropriate
functions.
Probably none.

I think that if the majority of developers use testing functions like
these, it goes a long way towards forcing the browser makers to stay
on the beaten path. Unfortunately, at the present time, most
developers are still addicted to browser sniffing, which encourages
browser makers to deploy creative countermeasures.
[snip]
I firmly believe that each of the three functions has its place.
Using each as appropriate makes the intentions of gateway code clear.
Future maintenance may be needed for any of the three, but I wouldn't
assume that such maintenance would be avoidable if all three are
lumped together (in fact, it may require them to be broken up, which
would require rewriting all of the calling code.)

It is true.

I am glad we are in agreement. These are important issues, certainly
worthy of debate, but it was starting to seem like we would never
reach a common ground.
 
P

Peter Michaux

One of the primary goals of the tabbed pane example is to allow for
the tentative styling of the widget if the widget might work. This is
because HTML does incremental rendering as the document loads and the
user should see something pleasing during that potentially long period
of time.

The basic pattern is this

---------

if (tests that can run during initial JavaScript execution pass) {

add a stylesheet that gives tentative and final stylings
in both cases where the widget is enabled or not

addDomReadyListener(function() {

if (final tests that need the full DOM pass) {
enabled the widget
}
else {
disabled the widget
}

})

}

--------

My big concern has been that if I add the tentative styling before
final testing, the final testing may never run because the DOM ready
listeners may not fire. I was confusing mixing in the Safari/anchor/
click/addEventListener/preventDefault problem into this by thinking I
would use DOM0 handlers for all kinds of events.

If the addDomReadyListener function is using either addEventListener
or attachEvent, then I can know in the first set of tests (i.e. in the
outer "if" above) that the domReadyListeners will run. There aren't
any known problems with this. Now I'm satisfied that I can at least
"get out of trouble".

Peter
 
T

Thomas 'PointedEars' Lahn

Peter said:
Below I've included the four files involved in the example:
[ca. 500 lines of code]

Ahh, *there's* the K key.


F'up2 poster

PointedEars
 
P

Peter Michaux

Peter said:
Below I've included the four files involved in the example:
[ca. 500 lines of code]

Ahh, *there's* the K key.

I'm glad you are learning the keybindings of your editor. I'm sure you
will be a much more efficient computer user now. Thanks for sharing.

F'up2 poster

I've told you that I don't know the uncommon abbreviations you insist
on using.

Peter
 
P

Peter Michaux

Below is my code for the event part of the tabbed pane example. I'm
pretty happy with this. It is much more compact than other workarounds
that solve the Safari problem. It would be easy to add a
LIB.removeListener function to the following code. I don't need that
for the tabbed pane demo.

This solves the Safari problem. I tested on S2.0 this morning and will
check 1.x versions later today.) The more I have thought about
cancelBubble the more I think using it is just a bad practice anyway
so the fact that the Safari workaround breaks if cancelBubble is used
doesn't bother me at all.

IE circular reference leaks should be handled properly. I think I have
done so. I don't know exactly why I continue a tradition of setting
some variables to null in an event module as part of the clean up. I
don't think these lines are necessary.

I don't care in which order event handlers run and don't think making
sure they run in the order they are added encourages good programming
or is worth a single character.

David, I looked at some of your event code and noticed that you don't
use DOMContentLoaded with IE. I think that if IE gets their act
together they will probably have an 'onDOMContentLoaded' event so I
have used it below.

Peter

// -----------------------------

(function() {

if (Function.prototype.apply &&
LIB.getAnElement) {

var global = this,
anElement = LIB.getAnElement(),
listeners = [];

var wrapHandler = function(element, handler, options) {
options = options || {};
var thisObj = options.thisObj || element;
return function(e) {
return handler.apply(thisObj, [e || global.event]);
};
};

var createListener = function(element, eventType,
handler, options) {
return {
element: element,
eventType: eventType,
handler: handler,
wrappedHandler: wrapHandler(element, handler, options)
};
};

// BEGIN DOM2 event model
if (LIB.isHostMethod(global, 'addEventListener') &&
LIB.isHostMethod(anElement, 'addEventListener')) {

LIB.addListener = function(element, eventType,
handler, options) {
var listener = createListener(element, eventType,
handler, options);
element.addEventListener(
listener.eventType,
listener.wrappedHandler,
false);
listeners[listeners.length] = listener;
};

// Safari preventDefault workaround
if (LIB.getDocumentElement) {
var docElement = LIB.getDocumentElement();

var prevented = false;

// Clobbers any HTML inline element but it is
// trivial to build a workaround for that.
// The reason to use the docElement is because
// it is the least likely place where clobbering
// will cause an accidental problem.
docElement.onclick =
docElement.ondblclick =
function() {
var result = !prevented;
prevented = false;
return result;
};

// clean up for memory leaks
// No known problems but just being careful
docElement = null;
}

// event must bubble up to documentElement
// so no cancelBubble. It is a bad practice
// anyway because canceling bubble foils
// unrelated delegate listeners
LIB.preventDefault = function(e) {
prevented = true;
if (LIB.isHostMethod(e, 'preventDefault')) {
e.preventDefault();
}
};

} // END DOM2 event model
// BEGIN IE event model
else if (LIB.isHostMethod(global, 'attachEvent') &&
LIB.isHostMethod(anElement, 'attachEvent')
// uncomment next line if using
// the try-catch below
//&& global.setTimeout
) {

// IE leaks memory and want to cleanup
// before when the document unloads
var unloadListeners = [];

LIB.addListener = function(element, eventType,
handler, options) {
var listener = createListener(element, 'on'+eventType,
handler, options);

if (eventType == 'unload') {
unloadListeners[unloadListeners.length] = listener;
}
else {
element.attachEvent(
listener.eventType,
listener.wrappedHandler);
listeners[listeners.length] = listener;
}
};

global.attachEvent(
'onunload',
function(e) {
e = e || window.event;
var i, listener;
for (i=unloadListeners.length; i--; ) {
listener = unloadListeners;
if (listener) {
// Uncomment try-catch if you do not need to support
// old browsers or some modern cell phones.
// The try-catch will ensure all handlers run.
// Also uncomment test for global.setTimeout above
//try {
listener.wrappedHandler(e);
//}
//catch (err) {
// (function(err) {
// global.setTimeout(function() {throw err}, 10);
// })(err);
//}
}
}
global.detachEvent(
'onunload',
arguments.callee);

for (i=listeners.length; i-- ; ) {
listener = listeners;
if (listener) {
// break circular reference to
// fix IE memory leak
listener.element.detachEvent(
listener.eventType,
listener.wrappedHandler);
// next lines are just to help
// garbage collection. No known
// problems.
listener.element =
listener.handler =
listener.wrappedHandler =
null;
listeners = null;
}
}
}
);

LIB.preventDefault = function(e) {
e.returnValue = false;
};

} // END IE event model


if (LIB.addListener &&
LIB.getDocumentElement) {

var readyListeners = [];

LIB.addDomReadyListener = function(handler, options) {
var options = options || {};
readyListeners[readyListeners.length] =
wrapHandler(
options.docNode || LIB.getDocumentElement(),
handler,
options);
}

var bReady = false;
var handleDomReady = function() {
if (!bReady) {
bReady = true;
for (var i = readyListeners.length; i--; ) {
readyListeners();
}
}
};
LIB.fireDomReady = handleDomReady;
LIB.addListener(global, 'DOMContentLoaded', handleDomReady);
LIB.addListener(global, 'load', handleDomReady);
}

// clean up for memory leaks
// no known problems
anElement = null;
}
})();
 
D

David Mark

Below is my code for the event part of the tabbed pane example. I'm
pretty happy with this. It is much more compact than other workarounds
that solve the Safari problem. It would be easy to add a
LIB.removeListener function to the following code. I don't need that
for the tabbed pane demo.

This solves the Safari problem. I tested on S2.0 this morning and will
check 1.x versions later today.) The more I have thought about
cancelBubble the more I think using it is just a bad practice anyway
so the fact that the Safari workaround breaks if cancelBubble is used
doesn't bother me at all.

I can't think of a recent project where I needed to cancel event
bubbling.
IE circular reference leaks should be handled properly. I think I have
done so. I don't know exactly why I continue a tradition of setting
some variables to null in an event module as part of the clean up. I
don't think these lines are necessary.

It is often necessary to prevent the circular references that cause IE
memory leaks.
I don't care in which order event handlers run and don't think making

You don't need to worry about that. The standard explicitly allows
for any arbitrary firing order. Code that attempts to predict the
order is broken by design.
sure they run in the order they are added encourages good programming
or is worth a single character.

It would clearly be a waste of time.
David, I looked at some of your event code and noticed that you don't
use DOMContentLoaded with IE. I think that if IE gets their act
together they will probably have an 'onDOMContentLoaded' event so I
have used it below.

Do you mean I don't use it with attachEvent? That sounds like an
oversight. However, I am hopeful that IE8 will implement
addEventListener and not attempt to fix attachEvent. Additionally, if
they do implement addEventListener, I hope they will follow the
standards. It won't make a difference for my code (other than the
oversight you pointed out), but it could wreak havoc on existing
sites.

[snip]
// -----------------------------

(function() {

  if (Function.prototype.apply &&
      LIB.getAnElement) {

      var global = this,
          anElement = LIB.getAnElement(),
          listeners = [];

      var wrapHandler = function(element, handler, options) {
        options = options || {};
        var thisObj = options.thisObj || element;
        return function(e) {
          return handler.apply(thisObj, [e || global.event]);
        };

Why not use call here? And do you need to "wrap" handlers for DOM2
implementations? I only do it if an optional context is specified
(i.e. "this" needs to be set to something other than the element at
hand.)
      };

      var createListener = function(element, eventType,
                                             handler, options) {
        return {
          element: element,
          eventType: eventType,
          handler: handler,
          wrappedHandler: wrapHandler(element, handler, options)
        };
      };

    // BEGIN DOM2 event model
    if (LIB.isHostMethod(global, 'addEventListener') &&
        LIB.isHostMethod(anElement, 'addEventListener')) {

Watch out here. There is no standard for the window object and it has
been reported that some agents don't bother to implement
addEventListener for that object.
      LIB.addListener = function(element, eventType,
                                               handler, options) {
        var listener = createListener(element, eventType,
                                                handler, options);
        element.addEventListener(
          listener.eventType,
          listener.wrappedHandler,
          false);
        listeners[listeners.length] = listener;

Make sure that the listeners array does not end up sharing a closure
with this code (as it does here.) I assume that unload cleanup logic
will follow at some point, but AIUI, such code doesn't do a bit of
good for elements that have been removed from their documents (e.g. by
innerHTML replacement.) IIRC, you covered this in your FORK library.
      };

      // Safari preventDefault workaround
      if (LIB.getDocumentElement) {
        var docElement = LIB.getDocumentElement();

        var prevented = false;

        // Clobbers any HTML inline element but it is
        // trivial to build a workaround for that.
        // The reason to use the docElement is because
        // it is the least likely place where clobbering
        // will cause an accidental problem.

Makes sense.
        docElement.onclick =
        docElement.ondblclick =
          function() {
            var result = !prevented;
            prevented = false;
            return result;
          };

        // clean up for memory leaks
        // No known problems but just being careful

Skipping this step would create a potential for leakage in IE. This
would only be an issue if the code that normalizes and attaches events
shares a closures with this reference. Imagine if calling code
attached a listener to the documentElement.
        docElement = null;
      }

      // event must bubble up to documentElement
      // so no cancelBubble. It is a bad practice
      // anyway because canceling bubble foils
      // unrelated delegate listeners

That's certainly one reason not to use it.
      LIB.preventDefault = function(e) {
        prevented = true;
        if (LIB.isHostMethod(e, 'preventDefault')) {
          e.preventDefault();
        }
      };

    } // END DOM2 event model
    // BEGIN IE event model
    else if (LIB.isHostMethod(global, 'attachEvent') &&
             LIB.isHostMethod(anElement, 'attachEvent')
             // uncomment next line if using
             // the try-catch below
             //&& global.setTimeout
             ) {

      // IE leaks memory and want to cleanup
      // before when the document unloads
      var unloadListeners = [];

      LIB.addListener = function(element, eventType,
                                               handler, options) {
        var listener = createListener(element, 'on'+eventType,
                                                handler, options);

        if (eventType == 'unload') {

Shouldn't this deal only with unload listeners on the window object?
Are there even distinct unload events for DOM objects (e.g. body) or
are they all lumped together (as with body.onload?) If an
implementation does make a distinction, I would assume that the
window's unload listeners would be the last to fire.
          unloadListeners[unloadListeners.length] = listener;

This is a good idea and something I need to add to mine. Otherwise,
you can't clean up unload listeners (might remove them before they
have run.) I think I just left the window's unload listeners alone in
my most recent version. I am not sure whether listeners attached to
the window object can create memory leaks in IE. (?)
        }
        else {
          element.attachEvent(
            listener.eventType,
            listener.wrappedHandler);
          listeners[listeners.length] = listener;
        }
      };

      global.attachEvent(
        'onunload',
        function(e) {
          e = e || window.event;
          var i, listener;
          for (i=unloadListeners.length; i--; ) {
            listener = unloadListeners;
            if (listener) {
              // Uncomment try-catch if you do not need to support
              // old browsers or some modern cell phones.
              // The try-catch will ensure all handlers run.
              // Also uncomment test for global.setTimeout above
              //try {
                listener.wrappedHandler(e);


Shouldn't you call these?
              //}
              //catch (err) {
              //  (function(err) {
              //    global.setTimeout(function() {throw err}, 10);
              //  })(err);
              //}
            }
          }
          global.detachEvent(
            'onunload',
            arguments.callee);

          for (i=listeners.length; i-- ; ) {
            listener = listeners;
            if (listener) {
              // break circular reference to
              // fix IE memory leak
              listener.element.detachEvent(
                listener.eventType,
                listener.wrappedHandler);
              // next lines are just to help
              // garbage collection. No known
              // problems.
              listener.element =
              listener.handler =
              listener.wrappedHandler =
                null;
              listeners = null;


This last line should suffice. I assume nothing else references the
"listener" objects in the array.
            }
          }
        }
      );

      LIB.preventDefault = function(e) {
        e.returnValue = false;
      };

    } // END IE event model

    if (LIB.addListener &&
        LIB.getDocumentElement) {

      var readyListeners = [];

      LIB.addDomReadyListener = function(handler, options) {
        var options = options || {};
        readyListeners[readyListeners.length] =
          wrapHandler(
            options.docNode || LIB.getDocumentElement(),

This looks wrong. Should be:

options.docNode || global.document

However, as with my version, you only have one array for document
ready listeners (and one ready flag), so you cannot support multiple
documents here.

            handler,
            options);
      }

      var bReady = false;
      var handleDomReady = function() {
        if (!bReady) {
          bReady = true;
          for (var i = readyListeners.length; i--; ) {
            readyListeners();
          }
        }
      };
      LIB.fireDomReady = handleDomReady;


I wish we could agree on the naming of the various methods (e.g.
getDocumentElement vs. getHtmlElement, fireDomReady vs. whatever I
called that, etc.)
      LIB.addListener(global, 'DOMContentLoaded', handleDomReady);
      LIB.addListener(global, 'load', handleDomReady);
    }

    // clean up for memory leaks
    // no known problems

There could be a problem if an application attached an event to
whatever anElement references. Your wrapped function's scope would
include anElement. Of course, the cleanup code should clear things up
on unload, but shouldn't be counted on. To this end, I made my
"attachedListeners" array, as well as an array that stores contexts
for each listener, properties of the global API object.

That reminds me, both versions will grow infinitely long arrays if
listeners are attached and detached repeatedly. A good example of a
module that does this is drag and drop, which temporarily attaches
mouseup, mousemove and keyup listeners (for keyboard-controlled drag
operations) to the document object. Each time an element is dragged,
the "cleanup queue" gets a little bigger. Adding a splice operation
to prevent that is at the top of my list.
 
P

Peter Michaux

Do you mean I don't use it with attachEvent?

It looks like you do not. I see three in your code

docNode.addEventListener('DOMContentLoaded', documentReadyListener,
false);
global.addEventListener('load', documentReadyListener, false);
global.attachEvent('onload', documentReadyListener);

That sounds like an
oversight. However, I am hopeful that IE8 will implement
addEventListener

I don't think they will. It is in their best interest to make sure
browsers are not interchangeable commodities. They have corporate
environments with intranets for IE only locked in.

(function() {
if (Function.prototype.apply &&
LIB.getAnElement) {
var global = this,
anElement = LIB.getAnElement(),
listeners = [];
var wrapHandler = function(element, handler, options) {
options = options || {};
var thisObj = options.thisObj || element;
return function(e) {
return handler.apply(thisObj, [e || global.event]);
};

Why not use call here?

Apply is older and works so there is I figured there was no need to
raise the bar in this case.

I also usually have an options.args array the caller can supply and so
that needs to be dealt with using apply and concatenating the event
onto the front.
And do you need to "wrap" handlers for DOM2
implementations? I only do it if an optional context is specified
(i.e. "this" needs to be set to something other than the element at
hand.)

In my options object I allow for parameters to to be passed to the
handler function. So in general it needs to be wrapped and I don't
look for the situations where I don't need to wrap it.

Watch out here. There is no standard for the window object and it has
been reported that some agents don't bother to implement
addEventListener for that object.

I suppose I'm willing to let those one fall into the graceful
degradation path. They aren't common.

LIB.addListener = function(element, eventType,
handler, options) {
var listener = createListener(element, eventType,
handler, options);
element.addEventListener(
listener.eventType,
listener.wrappedHandler,
false);
listeners[listeners.length] = listener;

Make sure that the listeners array does not end up sharing a closure
with this code (as it does here.) I assume that unload cleanup logic
will follow at some point, but AIUI, such code doesn't do a bit of
good for elements that have been removed from their documents (e.g. by
innerHTML replacement.) IIRC, you covered this in your FORK library.

I don't really follow and these circular references make my head hurt.

I don't have any unload cleanup for DOM2 as the only circular memory
leak browser I know of is IE.

I could add the IE unload listener cleanup just to be cautious.

[snip]
// BEGIN IE event model
else if (LIB.isHostMethod(global, 'attachEvent') &&
LIB.isHostMethod(anElement, 'attachEvent')
// uncomment next line if using
// the try-catch below
//&& global.setTimeout
) {
// IE leaks memory and want to cleanup
// before when the document unloads
var unloadListeners = [];
LIB.addListener = function(element, eventType,
handler, options) {
var listener = createListener(element, 'on'+eventType,
handler, options);
if (eventType == 'unload') {

Shouldn't this deal only with unload listeners on the window object?

Yes.

Since this library can't deal with multiple windows I could change to

if (eventType == 'unload' && element == global) {
Are there even distinct unload events for DOM objects (e.g. body) or
are they all lumped together (as with body.onload?)

I think in the standard body and frameset have them.

If an
implementation does make a distinction, I would assume that the
window's unload listeners would be the last to fire.
unloadListeners[unloadListeners.length] = listener;

This is a good idea and something I need to add to mine. Otherwise,
you can't clean up unload listeners (might remove them before they
have run.) I think I just left the window's unload listeners alone in
my most recent version. I am not sure whether listeners attached to
the window object can create memory leaks in IE. (?)

Given the oddities of IE, could anyone say he is sure?

}
else {
element.attachEvent(
listener.eventType,
listener.wrappedHandler);
listeners[listeners.length] = listener;
}
};
global.attachEvent(
'onunload',
function(e) {
e = e || window.event;
var i, listener;
for (i=unloadListeners.length; i--; ) {
listener = unloadListeners;
if (listener) {
// Uncomment try-catch if you do not need to support
// old browsers or some modern cell phones.
// The try-catch will ensure all handlers run.
// Also uncomment test for global.setTimeout above
//try {
listener.wrappedHandler(e);


Shouldn't you call these?


When they are wrapped the "this" object is bound. The wrapping is like
"partial application".

[snip]
LIB.addDomReadyListener = function(handler, options) {
var options = options || {};
readyListeners[readyListeners.length] =
wrapHandler(
options.docNode || LIB.getDocumentElement(),

This looks wrong. Should be:

options.docNode || global.document

It is very wrong.


However, as with my version, you only have one array for document
ready listeners (and one ready flag), so you cannot support multiple
documents here.

Indeed. I will also remove the options.docNode part.

I will have

wrapHandler(global.document, ...)

handler,
options);
}
var bReady = false;
var handleDomReady = function() {
if (!bReady) {
bReady = true;
for (var i = readyListeners.length; i--; ) {
readyListeners();
}
}
};
LIB.fireDomReady = handleDomReady;


I wish we could agree on the naming of the various methods (e.g.
getDocumentElement vs. getHtmlElement


getHtmlElement would be ok if you never wanted to have the same API
with XML documents. I think that is why I use getDocumentElement.
fireDomReady vs. whatever I
called that, etc.)

"fire" is just so action packed I can't resist.

There could be a problem if an application attached an event to
whatever anElement references. Your wrapped function's scope would
include anElement. Of course, the cleanup code should clear things up
on unload, but shouldn't be counted on. To this end, I made my
"attachedListeners" array, as well as an array that stores contexts
for each listener, properties of the global API object.

That reminds me, both versions will grow infinitely long arrays if
listeners are attached and detached repeatedly. A good example of a
module that does this is drag and drop, which temporarily attaches
mouseup, mousemove and keyup listeners (for keyboard-controlled drag
operations) to the document object. Each time an element is dragged,
the "cleanup queue" gets a little bigger. Adding a splice operation
to prevent that is at the top of my list.

Before firing the event listeners, it is necessary to make a copy of
them and iterate over the copy. This is because if one of the
listeners removes itself then the listener that was to be next is
skipped. Well that is if you are iterating forward through the list.
If you are iterating backwards through the list other problems can
occur. A copy is essential. Then you need to decide if a listener that
hasn't run is removed by another, does the removed listener run or
not.

Thanks for the detailed read through.

One thing I realized about my Safari workaround is that by having it
on the documentElement, if a click listener on the window did prevent
default I wouldn't catch that. So I have moved the workaround up to
the window level and do something like the IE unload listener
workaround keeping the click and dblclick listeners in an array and
looping over them when they need to run. This is still ok because at
least these aren't the mousemove events that need to run so
repetitively.

I'll do another couple passes through the code to try and get the last
details and then I think it will be ready.

One thing I have been thinking about changing is the use of internal
references to the libraries own functions. You frequently have
something like

function isHostMethod(){}
API.isHostMethod = isHostMethod

and then in your library code directly use "isHostMethod". I am
thinking about using the public API.isHostMethod even internally
because if a developer wants to wrap it, then I want that to affect
the libraries use also. This makes the library API extensible for
"plugins". Why they are called "plugins" I have no idea. This could
get into a whole debate about efficiency and early binding vs
flexibility and late binding. JavaScript itself is a late binding
language and I buy into the concept that the performance hit is worth
it. It is very minor anyway.

Peter
 
D

David Mark

It looks like you do not. I see three in your code

docNode.addEventListener('DOMContentLoaded', documentReadyListener,
false);
global.addEventListener('load', documentReadyListener, false);
global.attachEvent('onload', documentReadyListener);


I don't think they will. It is in their best interest to make sure
browsers are not interchangeable commodities. They have corporate
environments with intranets for IE only locked in.

Let's hope they at least fix get/setAttribute.
(function() {
  if (Function.prototype.apply &&
      LIB.getAnElement) {
      var global = this,
          anElement = LIB.getAnElement(),
          listeners = [];
      var wrapHandler = function(element, handler, options) {
        options = options || {};
        var thisObj = options.thisObj || element;
        return function(e) {
          return handler.apply(thisObj, [e || global.event]);
        };
Why not use call here?

Apply is older and works so there is I figured there was no need to

Older? IIRC, they were both introduced in JS 1.3.
raise the bar in this case.

I also usually have an options.args array the caller can supply and so
that needs to be dealt with using apply and concatenating the event
onto the front.
And do you need to "wrap" handlers for DOM2
implementations?  I only do it if an optional context is specified
(i.e. "this" needs to be set to something other than the element at
hand.)

In my options object I allow for parameters to to be passed to the
handler function. So in general it needs to be wrapped and I don't
look for the situations where I don't need to wrap it.
Watch out here.  There is no standard for the window object and it has
been reported that some agents don't bother to implement
addEventListener for that object.

I suppose I'm willing to let those one fall into the graceful
degradation path. They aren't common.
      LIB.addListener = function(element, eventType,
                                               handler, options) {
        var listener = createListener(element, eventType,
                                                handler, options);
        element.addEventListener(
          listener.eventType,
          listener.wrappedHandler,
          false);
        listeners[listeners.length] = listener;
Make sure that the listeners array does not end up sharing a closure
with this code (as it does here.)  I assume that unload cleanup logic
will follow at some point, but AIUI, such code doesn't do a bit of
good for elements that have been removed from their documents (e.g. by
innerHTML replacement.)  IIRC, you covered this in your FORK library.

I don't really follow and these circular references make my head hurt.

If your array that holds references to the elements with attached
listeners shares a closure with the wrapped listener functions, you
will create a new circular reference every time.
I don't have any unload cleanup for DOM2 as the only circular memory
leak browser I know of is IE.

Yes, but apply this to the IE branch, which also stores element
references (two references each as I recall, one to keep track of the
listeners per element and one for cleanup.)
I could add the IE unload listener cleanup just to be cautious.

That wasn't what I was getting at. I don't do the unload cleanup
unless attachEvent or DOM0 is in use. Attaching unload listeners is
best avoided when possible (disables fast history navigation in most
browsers.)
[snip]
    // BEGIN IE event model
    else if (LIB.isHostMethod(global, 'attachEvent') &&
             LIB.isHostMethod(anElement, 'attachEvent')
             // uncomment next line if using
             // the try-catch below
             //&& global.setTimeout
             ) {
      // IE leaks memory and want to cleanup
      // before when the document unloads
      var unloadListeners = [];
      LIB.addListener = function(element, eventType,
                                               handler, options) {
        var listener = createListener(element, 'on'+eventType,
                                                handler, options);
        if (eventType == 'unload') {
Shouldn't this deal only with unload listeners on the window object?

Yes.

Since this library can't deal with multiple windows I could change to

if (eventType == 'unload' && element == global) {

That is similar to what I did for this case.
I think in the standard body and frameset have them.

But aren't they just mapped to the window unload event? An onload
attribute on the body can be overwritten by assigning something to
window.onload. Is this not the same for unload?
If an
implementation does make a distinction, I would assume that the
window's unload listeners would be the last to fire.
          unloadListeners[unloadListeners.length] = listener;
This is a good idea and something I need to add to mine.  Otherwise,
you can't clean up unload listeners (might remove them before they
have run.)  I think I just left the window's unload listeners alone in
my most recent version.  I am not sure whether listeners attached to
the window object can create memory leaks in IE. (?)

Given the oddities of IE, could anyone say he is sure?

Maybe Bill Gates can chime in on this. I have always heard that
references to DOM objects must be involved and the window object is
clearly not a DOM object. Regardless, I avoid having references to
the window (global) object inside closures.
        }
        else {
          element.attachEvent(
            listener.eventType,
            listener.wrappedHandler);
          listeners[listeners.length] = listener;
        }
      };
      global.attachEvent(
        'onunload',
        function(e) {
          e = e || window.event;
          var i, listener;
          for (i=unloadListeners.length; i--; ) {
            listener = unloadListeners;
            if (listener) {
              // Uncomment try-catch if you do not need to support
              // old browsers or some modern cell phones..
              // The try-catch will ensure all handlers run.
              // Also uncomment test for global.setTimeout above
              //try {
                listener.wrappedHandler(e);

Shouldn't you call these?

When they are wrapped the "this" object is bound. The wrapping is like
"partial application".


Oops. I missed that these were "wrapped" listener functions.
[snip]
      LIB.addDomReadyListener = function(handler, options) {
        var options = options || {};
        readyListeners[readyListeners.length] =
          wrapHandler(
            options.docNode || LIB.getDocumentElement(),
This looks wrong.  Should be:
options.docNode || global.document

It is very wrong.

I thought I was seeing things at first.
However, as with my version, you only have one array for document
ready listeners (and one ready flag), so you cannot support multiple
documents here.

Indeed. I will also remove the options.docNode part.

I will have

wrapHandler(global.document, ...)
            handler,
            options);
      }
      var bReady = false;
      var handleDomReady = function() {
        if (!bReady) {
          bReady = true;
          for (var i = readyListeners.length; i--; ) {
            readyListeners();
          }
        }
      };
      LIB.fireDomReady = handleDomReady;

I wish we could agree on the naming of the various methods (e.g.
getDocumentElement vs. getHtmlElement

getHtmlElement would be ok if you never wanted to have the same API
with XML documents. I think that is why I use getDocumentElement.


Makes sense. I do support XML elements, but clearly getHtmlElement
has no part in that. I figured that callers would reference the
documentElement property directly for those. Remember, there are
alternate versions of getDocumentElement in CWR (at least I think they
made it into the repository.) The alternates use gEBTN or the all
property to find the HTML element.
"fire" is just so action packed I can't resist.

It appears I called my variation "documentReadyListener." It really
isn't an action at all (at least not in this example.) Where it is
(optionally) called directly (from the tail end of the markup), the
"fire" prefix makes more sense.
Before firing the event listeners, it is necessary to make a copy of
them and iterate over the copy. This is because if one of the

Is this in reference to the previously quoted bit? If so, you lost
me.
listeners removes itself then the listener that was to be next is
skipped. Well that is if you are iterating forward through the list.
If you are iterating backwards through the list other problems can
occur. A copy is essential. Then you need to decide if a listener that
hasn't run is removed by another, does the removed listener run or
not.

I don't think it is a good idea for listeners to remove themselves
during an event. In fact, I seem to remember some odd results with
code like that (can't remember the details, but I think it involved
NN6.2.) Nevertheless, my event scheme doesn't have this problem. At
first glance, I didn't think yours did either. You attach your
"wrapped" listeners as normal right (other than unload and DOM ready?)
Thanks for the detailed read through.

One thing I realized about my Safari workaround is that by having it
on the documentElement, if a click listener on the window did prevent
default I wouldn't catch that. So I have moved the workaround up to
the window level and do something like the IE unload listener
workaround keeping the click and dblclick listeners in an array and
looping over them when they need to run. This is still ok because at
least these aren't the mousemove events that need to run so
repetitively.

I would be careful about assuming anything about the window object.
Can it really be expected to receive click events in all (or even
most) implementations?
I'll do another couple passes through the code to try and get the last
details and then I think it will be ready.

One thing I have been thinking about changing is the use of internal
references to the libraries own functions. You frequently have
something like

function isHostMethod(){}
API.isHostMethod = isHostMethod

and then in your library code directly use "isHostMethod". I am
thinking about using the public API.isHostMethod even internally

I wouldn't do that for numerous reasons (performance for one.)
because if a developer wants to wrap it, then I want that to affect

I have an AOP-like debugger that wraps the methods of the API object.
I like that it only reports on calls from the outside.
the libraries use also. This makes the library API extensible for

It still could be extensible in this way, but it would be a less
flexible (and safer IMO) extensibility. Adding custom code inside the
library's closure to augment internal functions would allow what you
want (my higher-level modules do this repeatedly, see positionElement,
sizeElement, showElement, setElementHtml, changeImage, etc.) That is
the difference between a new module and an add-on (or plug-in or
whatever.)
"plugins". Why they are called "plugins" I have no idea. This could
get into a whole debate about efficiency and early binding vs
flexibility and late binding. JavaScript itself is a late binding
language and I buy into the concept that the performance hit is worth
it. It is very minor anyway.

I would avoid using LIB.* or API.* or whatever in the internal code.
Doing so also makes the code less portable.
 

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
474,091
Messages
2,570,605
Members
47,225
Latest member
DarrinWhit

Latest Threads

Top