fadeIn on inline elements: display css property ignored

Reported by:	JorisDebonnet	Owned by:
Priority:	high	Milestone:	1.7
Component:	effects	Version:	1.7b2
Keywords:		Cc:
Blocked by:		Blocking:

Description

Greetings,

in 1.7b2, contrary to 1.6.4, my inline elements lose their stylesheet-set display property after fading back in. It only happens if the css rule has a selector of at least two parts.

http://jsfiddle.net/AYeh4/

Attachments (0)

Change History (6)

Changed October 19, 2011 06:07PM UTC by timmywil comment:1

component:	unfiled → effects
milestone:	None → 1.7
priority:	undecided → high
status:	new → open

Changed October 25, 2011 05:36PM UTC by JorisDebonnet comment:2

Just for the record: the problem is still there in 1.7rc1 (although I suppose you already knew).

Changed October 31, 2011 10:57PM UTC by omni5cience comment:3

This has nothing to do with the selector having multiple parts. The reason the test case works when the style applied is span { display: block } and not when it's .a span { display: block } is because the former changes what's returned by defaultDisplay() so this bug is actually a duplicate of #10622.

We rewrote the show method, because it seemed unclear. We also wrote a test case for this ticket, which this patch fixes. All the tests pass but we're not sure if we've broken anything else. Also, we're not sure if this code is very performant.

Could somebody do a code review?

https://github.com/omni5cience/jquery/commit/3fe7e2dc0a1e09c67c1b37d93b5ae8cdb7734e4a

Changed November 01, 2011 05:07PM UTC by omni5cience comment:4

Replying to [comment:3 omni5cience]:

This has nothing to do with the selector having multiple parts. The reason the test case works when the style applied is span { display: block } and not when it's .a span { display: block } is because the former changes what's returned by defaultDisplay() so this bug is actually a duplicate of #10622. We rewrote the show method, because it seemed unclear. We also wrote a test case for this ticket, which this patch fixes. All the tests pass but we're not sure if we've broken anything else. Also, we're not sure if this code is very performant. Could somebody do a code review? https://github.com/omni5cience/jquery/commit/3fe7e2dc0a1e09c67c1b37d93b5ae8cdb7734e4a

I created a jsPerf test to evaluate the performance, and it turns out it's significantly slower, http://jsperf.com/jquery-show-testing I thought that the call to defaultDisplay() was what was slowing the loop down enough to trigger the page reflow, but I just realized that defaultDisplay already memoizes itself, so I was working under a flawed assumption.

I'm still not convinced that the loop can't be tightened enough to do what's done in two loops in one.

That said, I'm still curious what part of what I'm doing is slow.

Changed November 03, 2011 09:41PM UTC by JorisDebonnet comment:5

Well, can't say anything about the performance, but in any case in rc2 this issue is fixed.

Changed November 05, 2011 07:08PM UTC by timmywil comment:6

resolution:	→ worksforme
status:	open → closed

Thank you for looking into the problem further.

Side navigation

#10503 closed bug (worksforme)