Bug Tracker

Opened 11 years ago

Closed 11 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:



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 11 years ago by addyosmani

Component: unfiledeffects
Priority: undecidedlow
Status: newopen

comment:2 Changed 11 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 11 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 11 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 11 years ago by Rick Waldron

Status: newassigned

comment:6 Changed 11 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 11 years ago by Rick Waldron

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


See, they REALLY do that :(

comment:8 Changed 11 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 11 years ago by mathias (previous) (diff)

comment:9 Changed 11 years ago by lrbabe

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