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

[Peter Michaux]

Older? IIRC, they were both introduced in JS 1.3.

and JScript 5.5 according to Thomas Lahn's page

<URL:http://pointedears.de/scripts/es-matrix/>

I really thought there was one browser I encountered where it had
apply and not call. Anyway apply is what I need in this case.

[snip]

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.

But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.

Yes, but apply this to the IE branch,

I use detachEvent on all the listeners in 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.)

As long as detachEvent is used, the problem should be solved
regardless of the number of places I have references to the the
handlers and wrapped handlers and if those have the element in the
closure. Just one break in the circle is enough, I thought.

[snip]

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?

I don't know.

[snip]

I don't think it is a good idea for listeners to remove themselves
during an event.

That is what happens at the end of a drag operation. When the mouseup
event occur both the mousemove and mouseup listeners are removed. At
least that is one way to do it.

[snip]

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?

Before using the Safari workaround I check that

typeof global.onclick == 'object'

before anything has been assigned to global.onclick. When a function
is assigned the typeof value is 'function'. That is true in at least
safari so it is a safe bet that the event will bubble up to the
window.

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

Portable to where?

Peter

 

[David Mark]

Older?  IIRC, they were both introduced in JS 1.3.

and JScript 5.5 according to Thomas Lahn's page

<URL:http://pointedears.de/scripts/es-matrix/>

I really thought there was one browser I encountered where it had
apply and not call. Anyway apply is what I need in this case.

[snip]

      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.

But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.

From what I have heard, that doesn't help for elements that have been
removed from the document. I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements. IIRC, your FORK library does
that. Perhaps in this case the memory is leaked as soon as the
element is replaced? I really need to run some tests to confirm this
theory.

Regardless, I just don't think it is good practice to create circular
references involving DOM elements in the library's closure. I see the
unload cleanup as strictly a safeguard for applications that create
such references in closures outside of the library.

I use detachEvent on all the listeners in the IE branch.


As long as detachEvent is used, the problem should be solved
regardless of the number of places I have references to the the
handlers and wrapped handlers and if those have the element in the
closure. Just one break in the circle is enough, I thought.

One break is enough, assuming it comes before the memory has been
irretrievably lost.

[snip]

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?

I don't know.

I don't know either, but I suspect it is the case. Again, I need to
run some tests so as not to rely on guesswork.

[snip]

I don't think it is a good idea for listeners to remove themselves
during an event.

That is what happens at the end of a drag operation. When the mouseup
event occur both the mousemove and mouseup listeners are removed. At
least that is one way to do it.

Good point. But getting back to the original issue, the "cleanup
queue" grows larger on each drag operation. This should be prevented
by splicing the array when listeners are removed. You should keep
this in mind when you add the code to remove listeners.

[snip]

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?

Before using the Safari workaround I check that

typeof global.onclick == 'object'

A null value I assume. This seems to be the norm in all but FF.

before anything has been assigned to global.onclick. When a function
is assigned the typeof value is 'function'. That is true in at least
safari so it is a safe bet that the event will bubble up to the
window.

Makes sense.

Portable to where?

To other projects that do not involve the library.

 

[Peter Michaux]

[snip]

But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.

From what I have heard, that doesn't help for elements that have been
removed from the document. I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements. IIRC, your FORK library does
that.

Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.

Perhaps in this case the memory is leaked as soon as the
element is replaced? I really need to run some tests to confirm this
theory.

Regardless, I just don't think it is good practice to create circular
references involving DOM elements in the library's closure.

There is no choice when fixing the execution "this" object of a
handler. The handler has to have the element in it's closure.

Good point. But getting back to the original issue, the "cleanup
queue" grows larger on each drag operation. This should be prevented
by splicing the array when listeners are removed. You should keep
this in mind when you add the code to remove listeners.

It just needs to be done with care. Making a copy of the list of
listeners is how I did it in FORK. The DOM does this also. It is
necessary to determine the possible set of handlers before calling
them. If not and one handler is adding itself to the end of the list
of handlers, it could be an infinite loop.

The above is true in Safari before any function has been assigned to
global.onclick.

A null value I assume. This seems to be the norm in all but FF.




Makes sense.

Peter

 

[David Mark]

[snip]

But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.

From what I have heard, that doesn't help for elements that have been
removed from the document.  I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements.  IIRC, your FORK library does
that.

Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.

But why if they will ultimately be purged on unload? Such practices
are what have led me to believe that the unload purge will be
ineffective in some cases. As mentioned, it would be a better idea to
test this directly than to rely on such deductions.

There is no choice when fixing the execution "this" object of a
handler. The handler has to have the element in it's closure.

Sure there is. Store the context references outside of the closure
(e.g. in an array that is a property of the API object.) The function
that adds a new reference returns the length of the array minus one as
a handle for the wrapped function to use. That's how I did it anyway.

It just needs to be done with care. Making a copy of the list of
listeners is how I did it in FORK. The DOM does this also. It is
necessary to determine the possible set of handlers before calling
them. If not and one handler is adding itself to the end of the list
of handlers, it could be an infinite loop.

Are you speaking specifically about the unload event? I don't see
where else in your code (other than the DOM Ready, which really
doesn't need an exposed detach function) that you call listeners
directly (i.e. the wrapped listener functions are attached and
subsequently called by the DOM.) I did the same thing for DOM0 using
what I think is a unique strategy. I will elaborate on that when we
get to that topic.

 

[Peter Michaux]

[snip]

But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.
From what I have heard, that doesn't help for elements that have been
removed from the document. I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements. IIRC, your FORK library does
that.

Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.

But why if they will ultimately be purged on unload?

That is an excellent question. I would guess you could leave it until
onload and that being part of the DOM or not would not affect
collectablility. The reason I use the purgeElement is it is better to
clean up as soon as possible so that huge chunks of DOM can be garbage
collected before onunload. If the page is very long lived and the DOM
is heavily modified frequently you wouldn't want to let the garbage
build into a huge pile until onunload.

Sure there is. Store the context references outside of the closure
(e.g. in an array that is a property of the API object.) The function
that adds a new reference returns the length of the array minus one as
a handle for the wrapped function to use. That's how I did it anyway.

That is very indirect and sounds like there is no real performance
hit. By avoiding creating the circular reference you wouldn't need the
purgeElement as I discussed above for the libraries behavior. However,
I think I would still use purgElement because I may have been sloppy
in my application code. I don't want to have to think that hard and
take such special care to avoid circular leaks when I'm writing
application code.

Are you speaking specifically about the unload event?
Yes

I don't see
where else in your code (other than the DOM Ready, which really
doesn't need an exposed detach function) that you call listeners
directly (i.e. the wrapped listener functions are attached and
subsequently called by the DOM.)

For what I posted here that is the only place.

I did the same thing for DOM0 using
what I think is a unique strategy. I will elaborate on that when we
get to that topic.

Here is where I did this in Fork using slice.

<URL: http://dev.forkjavascript.org/trac/browser/trunk/public/javascripts/fork/event.js#L113>

I need to add this same functionality to my code in this thread.

Peter

 

[Peter Michaux]

[snip]
But for IE onunload I'm using detachEvent so the element no longerhas
a reference to the functions involved. That should take care of it..
From what I have heard, that doesn't help for elements that have been
removed from the document.  I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements.  IIRC, your FORK library does
that.
Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.

But why if they will ultimately be purged on unload?

That is an excellent question. I would guess you could leave it until
onload and that being part of the DOM or not would not affect
collectablility. The reason I use the purgeElement is it is better to
clean up as soon as possible so that huge chunks of DOM can be garbage
collected before onunload. If the page is very long lived and the DOM
is heavily modified frequently you wouldn't want to let the garbage
build into a huge pile until onunload.

Sure there is.  Store the context references outside of the closure
(e.g. in an array that is a property of the API object.)  The function
that adds a new reference returns the length of the array minus one as
a handle for the wrapped function to use.  That's how I did it anyway.

That is very indirect and sounds like there is no real performance
hit. By avoiding creating the circular reference

Wait a second...

API is global. Every closure in your library has all the global
variables in it's closure. So the wrapped function has API in its
closure and API has a reference to the element to which the wrapped
function is attached. Isn't the circle still there but just bigger?

Peter

 

[David Mark]

[snip]
But for IE onunload I'm using detachEvent so the element no longerhas
a reference to the functions involved. That should take care of it..
From what I have heard, that doesn't help for elements that have been
removed from the document.  I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements.  IIRC, your FORK library does
that.
Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.

But why if they will ultimately be purged on unload?

That is an excellent question. I would guess you could leave it until
onload and that being part of the DOM or not would not affect
collectablility. The reason I use the purgeElement is it is better to
clean up as soon as possible so that huge chunks of DOM can be garbage
collected before onunload. If the page is very long lived and the DOM
is heavily modified frequently you wouldn't want to let the garbage
build into a huge pile until onunload.

That makes sense.

That is very indirect and sounds like there is no real performance
hit. By avoiding creating the circular reference you wouldn't need the
purgeElement as I discussed above for the libraries behavior. However,

Yes you would still need it as there is no telling how many circular
references may have been created *outside* of the library (e.g. by the
calling code.)

I think I would still use purgElement because I may have been sloppy
in my application code. I don't want to have to think that hard and
Right.

take such special care to avoid circular leaks when I'm writing
application code.

That's what I was getting at.

[snip]

 

[David Mark]

[snip]
But for IE onunload I'm using detachEvent so the element no longer has
a reference to the functions involved. That should take care of it.
From what I have heard, that doesn't help for elements that have been
removed from the document.  I don't understand why it wouldn't help,
but it seems the justification for the "clean" functions that are
called when innerHTML replaces elements.  IIRC, your FORK library does
that.
Yes. When I remove or replace elements in the DOM I call the
FORK.purgeElement with the deep flag to remove all listeners.
But why if they will ultimately be purged on unload?

That is an excellent question. I would guess you could leave it until
onload and that being part of the DOM or not would not affect
collectablility. The reason I use the purgeElement is it is better to
clean up as soon as possible so that huge chunks of DOM can be garbage
collected before onunload. If the page is very long lived and the DOM
is heavily modified frequently you wouldn't want to let the garbage
build into a huge pile until onunload.

That is very indirect and sounds like there is no real performance
hit. By avoiding creating the circular reference

Wait a second...

API is global. Every closure in your library has all the global
variables in it's closure. So the wrapped function has API in its

I assume you mean that properties of the Global Object are within the
scope of the closures (which is expected of course.)

closure and API has a reference to the element to which the wrapped
function is attached. Isn't the circle still there but just bigger?

If that were the case then there would be no way to avoid these sorts
of circular references as IE makes every element with an ID a property
of the Global Object and clearly every bit of code in a script can see
them. Thankfully, it is not the case.

I am sure that Richard could explain this better, but AIUI:

el => wrapped listener => Variable Object => listeners array => object
in the array => el

That pattern will clearly leak memory in IE unless the chain of
references is broken at some point (typically on unload or through a
listener purge when an element is replaced in the document.)

Your theory seems to indicate that you could swap the above "Variable
Object" for "Global Object", but that doesn't work as the wrapped
listener (a nested function created in the API's closure) does not
*reference* the Global Object. All properties of the Global Object
are clearly within its scope, but that is not an issue.

 

[AKS]

I'm writing a blog article about building a decent tabbed pane widget
for a web page on the general web (anyone with a browser and internet
connection has access)...

I have read the article today. Thank you, Peter - you have done huge
work!
I've noticed that new regular expressions have to be created every
time in functions named -hasClass- and -removeClass-. Why not to
create another free variable, as in the case of function named -
compile-, which does use -cache- variable?

var regExpCache = {};

LIB.hasClass = function(el, className) {
var re = regExpCache[className] || (regExpCache[className] =
new RegExp('(^|\\s+)' + className + '(\\s+|$)'));
return re.test(el.className);
};

 

[Peter Michaux]

I have read the article today. Thank you, Peter - you have done huge
work!
I've noticed that new regular expressions have to be created every
time in functions named -hasClass- and -removeClass-. Why not to
create another free variable, as in the case of function named -
compile-, which does use -cache- variable?

I did think about doing something like this after I finished the
article but I wanted to run some performance tests to see what kind of
difference this would make. If the hasClass function is called many
times with the same class name (10000 times) then using the cache
makes a difference (30-50% time savings)

What offended me more in my code is the fact that I have the logic for
building the regular expression in three places. Using a cache allows
me to aggregate the regexp creation logic into one function without a
real performance hit.

Because I had a global flag on the className RegExp I ran into a
problem when I made this conversion because the regexp is being used
with test() multiple times. The problem is discussed in the following
post to the group.
<URL: http://groups.google.com/group/comp.lang.javascript/msg/a9523f0b06c4bdcc>


Thanks for taking a close look at the code and the feedback.


Peter