Bug Tracker

Opened 7 years ago

Closed 7 years ago

#10503 closed bug (worksforme)

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/

Change History (6)

comment:1 Changed 7 years ago by timmywil

Component: unfiledeffects
Milestone: None1.7
Priority: undecidedhigh
Status: newopen

comment:2 Changed 7 years ago by JorisDebonnet

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

comment:3 Changed 7 years ago by 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

comment:4 in reply to:  3 Changed 7 years ago by omni5cience

Replying to 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.

comment:5 Changed 7 years ago by JorisDebonnet

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

comment:6 Changed 7 years ago by timmywil

Resolution: worksforme
Status: openclosed

Thank you for looking into the problem further.

Note: See TracTickets for help on using tickets.