Skip to main content

Bug Tracker

Side navigation

#8996 closed bug (invalid)

Opened April 28, 2011 07:27AM UTC

Closed May 20, 2011 03:54PM UTC

`defaultDisplay()` may return inaccurate results

Reported by: mathias Owned by: mathias
Priority: low Milestone:
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.

Attachments (0)
Change History (9)

Changed April 28, 2011 09:19AM UTC by addyosmani comment:1

component: unfiledeffects
priority: undecidedlow
status: newopen

Changed April 28, 2011 03:44PM UTC by timmywil comment:2

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?

Changed April 28, 2011 03:53PM UTC by rwaldron comment:3

owner: → 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?

Changed April 28, 2011 05:27PM UTC by mathias comment:4

status: pendingnew

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

Changed May 07, 2011 07:42PM UTC by rwaldron comment:5

status: newassigned

Changed May 07, 2011 07:45PM UTC by rwaldron comment:6

@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; }

Changed May 07, 2011 07:47PM UTC by rwaldron comment:7

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

See, they REALLY do that :(

Changed May 08, 2011 12:09PM UTC by mathias comment:8

_comment0: 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.1304857628882833

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.

Changed May 20, 2011 03:54PM UTC by lrbabe comment:9

resolution: → invalid
status: assignedclosed