Bug Tracker

Opened 11 years ago

Closed 10 years ago

#4637 closed bug (invalid)

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:


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.

Change History (6)

comment:1 Changed 11 years ago by hasenstein

Suggestion for a fix:

  • remove the line
this[i].style.display = old
  • to the following if change the condition tp if (old === "none")

comment:2 Changed 11 years ago by hasenstein

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

comment:3 Changed 11 years ago by hasenstein

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)

comment:4 Changed 11 years ago by hasenstein

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)

comment:5 Changed 10 years ago by dmethvin

Component: unfiledfx

comment:6 Changed 10 years ago by Rick Waldron

Resolution: invalid
Status: newclosed

Changes over 1.4.2, 1.4.3, 1.4.4rc1 render this invalid

Note: See TracTickets for help on using tickets.