Skip to main content

Bug Tracker

Side navigation

#12904 closed bug (fixed)

Opened November 16, 2012 03:59AM UTC

Closed December 02, 2012 09:24AM UTC

Last modified December 08, 2012 11:07PM UTC

.show()ing CSS hidden elements fails permanently if iframes are hidden during first try

Reported by: maranomynet Owned by: maranomynet
Priority: undecided Milestone: None
Component: unfiled Version: 1.8.3
Keywords: Cc:
Blocked by: Blocking:
Description

Affects all versions of jQuery 1.8 in Firefox 16 (at least)

This is kind of an obscure bug and it's kind of hard to explain, but real easy to fix.

Here's a reduced, annotated demo: http://jsfiddle.net/qT4Tg/

with a proposed fix (patch) attached in a comment.

Attachments (0)
Change History (10)

Changed November 16, 2012 09:59AM UTC by maranomynet comment:1

I reduced the demo a bit more: http://jsfiddle.net/qT4Tg/1/

Changed November 16, 2012 10:50AM UTC by maranomynet comment:2

It occurred to me that the same happens if body or html are set to display:none; ... so that might require a second check before css_defaultDisplay caches the result.

Here's a quick patch for jQuery 1.8.3 showing how this can be accomplished:

7005,7011c7005,7015
< 		iframe = document.body.appendChild(
< 			iframe || jQuery.extend( document.createElement("iframe"), {
< 				frameBorder: 0,
< 				width: 0,
< 				height: 0
< 			})
< 		);
---
> 		if ( !iframe )
> 		{
> 			iframe = document.createElement("iframe");
> 			iframe.frameBorder = 0;
> 			iframe.width = 0;
> 			iframe.height = 0;
> 			// Hidden iframes always rport display as 'none' in Firefox 17
> 			// Note: Hidden body might still result in a hidden iframe
> 			iframe.style.cssText += "display: block !important;";
> 		}
> 		document.body.appendChild( iframe );
7028,7029c7032,7042
< 	// Store the correct default display
< 	elemdisplay[ nodeName ] = display;
---
> 	if ( display === "none"  &&  curCSS( document.body, "display" ) === "none" )
> 	{
> 		// document.body won't hide forever,
> 		// so let's go for "block" for now, and not cache the result.
> 		display = "block";
> 	}
> 	else
> 	{
> 		// Store the correct default display
> 		elemdisplay[ nodeName ] = display;
> 	}

Changed November 16, 2012 01:23PM UTC by dmethvin comment:3

owner: → maranomynet
status: newpending

How does this differ from the parent-hidden case, which we also do not handle? These are relatively expensive operations so it is better to have the user write code that doesn't expect us to do these things under the covers.

Changed November 16, 2012 03:31PM UTC by maranomynet comment:4

status: pendingnew

1. This is a clear regression in 1.8

2. It works perfectly in all browsers except Firefox.

3. Simply setting iframe to display:none; triggers this - even though the iframe is not the affected element's parent.

4. And even for the body hidden case - which is strictly speaking "parent-hidden" case - it still differs because it causes all future attempts to .show() the affected elements to fail - even after the body/html have been made visible.

...

Regarding the added operational expense, is not as great as you may think:

In iframe-hidden case we need to add a **single** operation iframe.style.cssText += "display: block !important;"; which is probably offset by changing from jQuery.extend() to direct property assignments.

In the body hidden case there's one extra curCSS(document.body,"display") invocation - that only happens in the highly unusual case when display==='none'.

Someone more clever than me might even come up with an even faster method for that...

Changed November 16, 2012 04:46PM UTC by dmethvin comment:5

Good points on the bug and impact, broken-forever is not good. Can you put together a pull request?

Changed November 17, 2012 01:38AM UTC by maranomynet comment:6

Changed November 17, 2012 02:19PM UTC by rwaldron comment:7

I'm curious about what lead to this "regression".

Changed November 17, 2012 04:28PM UTC by rwaldron comment:8

status: newpending

This behaviour exists as far back as 1.6.4 (what happened to all of our older versions on jsfiddle?)

1.6.4 http://jsfiddle.net/rwaldron/7htPw/

1.7.2 http://jsfiddle.net/rwaldron/K69P7/

The reason we use an iframe with height: 0, width: 0 and frameborder: 0 is because Firefox doesn't have "presentation" for an iframe with display: none.

I've adapted your solution to a more suitable patch that includes tests. The only change that's necessary to fix this issue is the addition of iframe.style.cssText = "display: block !important"; after the iframe is created. You can either update your patch or I can merge mine.

https://github.com/rwldrn/jquery/commit/501060eb31766d5721ac4bd8fb43d3c3fc6a9140

Changed December 02, 2012 09:24AM UTC by trac-o-bot comment:9

resolution: → invalid
status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Changed December 08, 2012 11:07PM UTC by Richard Gibson comment:10

resolution: invalidfixed

Fix #12904: Firefox defaultDisplay with body/iframe display:none. Report and solution by @maranomynet; test by @rwldrn.

Changeset: d343e6b9ed501052f1676694d5e53649c92e65a0