Ticket #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: | ||
| Blocking: | Blocked by: |
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.
Change History
comment:1 Changed 19 months ago by timmywil
- Priority changed from undecided to high
- Status changed from new to open
- Component changed from unfiled to effects
- Milestone changed from None to 1.7
comment:2 Changed 19 months ago by JorisDebonnet
Just for the record: the problem is still there in 1.7rc1 (although I suppose you already knew).
comment:3 follow-up: ↓ 4 Changed 19 months 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 19 months 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.
Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.