Ticket 5343 (bugs in show/hide)

Ticket 5343 (bugs in show/hide)


I filed this bug a few days ago. I just joined the group and saw Dave
Methvin's post about bug reports, saying we should discuss them here
first. So, belatedly, here goes. This is largely a reproduction of the
ticket, with a bit more explanation of exactly what the issues are.
There are two issues that I see.
First, hide and show use a jQuery.data item called "olddisplay". There
are no comments describing it (that I can find), but it /looks/ to me
like the idea is that hide should set it to the current style.display
value, and show should restore it (if set), or just set a default
value otherwise.
The trouble is, what hide actually does is:
var old = jQuery.data(this[i], "olddisplay");
if ( !old && old !== "none" )
jQuery.data(this[i], "olddisplay", jQuery.css(this[i],
"display"));
The test !old && old !== "none" is obviously wrong - old !== "none"
is /always/ true if !old is true. So this copies the current
style.display value only when olddisplay has never been set, and even
when the current style.display is "none".
But if my interpretation of the intent is correct, the code should
actually be:
var old = jQuery.css(this[i], "display");
if ( old && old !== "none" )
jQuery.data(this[i], "olddisplay", old);
The second problem comes in show. It sets style.display to
olddisplay||"". The problem here is that if you call hide on an
initially hidden element, olddisplay will be "none", and as a result
style.display will still be "none".
But, fortunately, show then goes on to test whether style.display is
"none" (my /guess/ is that this code is only needed because of the
first bug. But perhaps there are browsers where this is really
needed). In this case, show then tries to find a good default value by
creating an element of the same type, inserting it in document.body,
and looking at its display.style value.
And here's the second problem. If it does that to a "tr" element,
Firefox really doesnt like it (Im guessing it would like the tr to be
inside a tbody, inside a table). When you remove a tr who's parent is
document.body the entire page flashes, and any plugins that are loaded
get unloaded.
I have a nice demo of how it affects the google earth plugin here:
http://maps.myosotissp.com/bugs/show_hide.html
If you wait for the ge plugin to load, then click the "Show TR"
button, you'll see the nice flashing effect, followed by a message
stating "The Google Earth Plugin had an internal error. Try reloading
the page".
All I did was create a display:none tr element (properly wrapped in a
table), call hide() on it, and then call show() when you click the
button.
Mark