Bug Tracker

Opened 8 years ago

Closed 8 years ago

#8996 closed bug (invalid)

`defaultDisplay()` may return inaccurate results

Reported by: mathias Owned by: mathias
Priority: low Milestone: 1.next
Component: effects Version: git
Keywords: Cc:
Blocked by: Blocking:

Description

https://github.com/jquery/jquery/blob/c0450f3c2aab7af902ccf4ba0537088fc26f065d/src/effects.js#L582

The new defaultDisplay() only uses the iframe when display === "none" or display === "".

A simple CSS rule like div { display: inline; } or div { display: inline-block; }will cause defaultDisplay('div') to return 'inline'.

It would probably be safer to always use the iframe technique.

Change History (9)

comment:1 Changed 8 years ago by addyosmani

Component: unfiledeffects
Priority: undecidedlow
Status: newopen

comment:2 Changed 8 years ago by timmywil

Hmm, even though that is not the real default display, isn't that the desired behavior? Considering the user wants all divs to be display: inline, shouldn't we set to display: inline when showing?

comment:3 Changed 8 years ago by Rick Waldron

Owner: set to mathias
Status: openpending

Just to clarify, the defaultDisplay() commits that I made were actually to address this very issue and to "force" the desired behaviour.

I'm going to bounce this back to @mathias - can you provide a reduced test case that shows the behaviour you describe above?

comment:4 Changed 8 years ago by mathias

Status: pendingnew

timmywil: Good point. But then I don’t think it should be named defaultDisplay but rather desiredDisplay or something like that.

comment:5 Changed 8 years ago by Rick Waldron

Status: newassigned

comment:6 Changed 8 years ago by Rick Waldron

@timmywil & @mathias

the code that creates an iframe, etc will only run when show() or hide() come up with the wrong display state for an element, which will only happen when the user does something absurd like: span { display: none; }

comment:7 Changed 8 years ago by Rick Waldron

Also, this is awesome... someone has kindly provided us with a real case:

http://bugs.jquery.com/ticket/9106

See, they REALLY do that :(

comment:8 Changed 8 years ago by mathias

Feel free to close this bug as invalid, since defaultDisplay() actually works as intended (minor bugs like #8994 aside). At most, I would vouch to rename it to desiredDisplay() for clarity. Just say the word if you agree and I’ll add another commit to my pull request for #8994.

Last edited 8 years ago by mathias (previous) (diff)

comment:9 Changed 8 years ago by lrbabe

Resolution: invalid
Status: assignedclosed
Note: See TracTickets for help on using tickets.