Bug Tracker

#4637 closed bug (invalid)

Opened May 08, 2009 03:00PM UTC

Closed October 29, 2010 04:12AM UTC

show(): style is mistakenly set in the first loop

Reported by: hasenstein Owned by:
Priority: trivial Milestone: 1.4
Component: effects Version: 1.3.2
Keywords: Cc:
Blocked by: Blocking:
Description

jQuery 1.3.2 lines 3785..3823: the comment in #3815 says the second loop exists to avoid setting the display in the first one, where potentially lots of browser (display list) reflows could happen.

However, in line 3792 at the top of the first loop the style is already set directly, so that the second loop is not useless (it sets the correct value when the old value was "none"), but it does not fulfill its other stated purpose of preventing the reflows.

Attachments (0)
Change History (6)

Changed May 08, 2009 03:07PM UTC by hasenstein comment:1

Suggestion for a fix:

  • remove the line

this[i].style.display = old || "";

  • to the following if change the condition tp

if (old === "none")

Changed May 08, 2009 03:09PM UTC by hasenstein comment:2

Well, there are some typos above and the ticket system swallowed the vertical bar characters, but you know what I mean :-)

Changed May 08, 2009 03:11PM UTC by hasenstein comment:3

Actually, I forgot: the first line in the first loop

var old = jQuery.data(this[i], "olddisplay");

should become

var old = jQuery.data(this[i], "olddisplay") OR "";

(written here with "OR" instead of the vertical bars which get swallowed by the ticket system)

Changed May 08, 2009 03:26PM UTC by hasenstein comment:4

Thinking about it some more, this is not correct since olddisplay is never set to "none" (it is set in hide()).

So since it cannot be "none", and if unset is set to "" in the current code, how can the subsequent style check result in "none"?

Okay, the creation of a temporary element is clear to me, to find out what the default value of "display" is for the given tag if none is given. What is not clear to me now is how we ever get there, since as I read the code that code is executed only if setting style.display="" results in it becoming "none" when reading the property afterwards?

BREAK... more issues (is it me or the code?)

Reading hide() I'm even more confused: what does

if ( !old && old !== "none" )

mean? The second part old!==="none" is reached only if old does not exist??? (not a truthy value)

Changed June 13, 2010 02:43AM UTC by dmethvin comment:5

component: unfiled → fx

Changed October 29, 2010 04:12AM UTC by rwaldron comment:6

resolution: → invalid
status: new → closed

Changes over 1.4.2, 1.4.3, 1.4.4rc1 render this invalid