Bug 3581

Summary: iFrames set to display:none are Missing from frames array
Product: WebKit Reporter: David Wheeler <david>
Component: JavaScriptCoreAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, ap, ian
Priority: P2    
Version: 412   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 9610    
Attachments:
Description Flags
Test Case
none
Screen shot of Safari rendering this test case
none
Screen shot of Mac Firefox 1.0.4 rendering this test case
none
Patch
andersca: review-
Better patch
none
Patch without tabs mjs: review+

David Wheeler
Reported 2005-06-16 17:02:48 PDT
* SUMMARY iframes with their CSS set to "display:none" are not included in the frames array but should be. * STEPS TO REPRODUCE 1. open testcase 2. Note that it displays "1" 3. Edit testcase to remove 'style="display:none"'. 4. Reload testcase. 5. Note that it now displays "2". * RESULTS There are two iframes in the HTML, but only one appears in the frames array. * EXPECTED RESULTS Both iframes should appear in the frames array, even if one is set to "display:none". Such is the case in Firefox.
Attachments
Test Case (230 bytes, text/plain)
2005-06-16 17:03 PDT, David Wheeler 		no flags
Details
Screen shot of Safari rendering this test case (79.26 KB, image/png)
2005-06-17 09:04 PDT, Chris Petersen 		no flags
Details
Screen shot of Mac Firefox 1.0.4 rendering this test case (48.78 KB, image/png)
2005-06-17 09:06 PDT, Chris Petersen 		no flags
Details
Patch (39.73 KB, patch)
2006-07-05 11:28 PDT, Anders Carlsson 		andersca: review-
DetailsFormatted DiffDiff
Better patch (40.93 KB, patch)
2006-07-05 13:18 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Patch without tabs (40.96 KB, patch)
2006-07-05 13:32 PDT, Anders Carlsson 		mjs: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
David Wheeler
Comment 1 2005-06-16 17:03:13 PDT
Created attachment 2410 [details] Test Case
Chris Petersen
Comment 2 2005-06-17 09:02:18 PDT
I see this issue on Safari 2.0 (v412) and TOT WebKit under 10.4.1(8B15). I have attached a screen shot of what is displayed under Safari 2.0 and Firefox 1.0.4.
Chris Petersen
Comment 3 2005-06-17 09:04:56 PDT
Created attachment 2440 [details] Screen shot of Safari rendering this test case
Chris Petersen
Comment 4 2005-06-17 09:06:16 PDT
Created attachment 2441 [details] Screen shot of Mac Firefox 1.0.4 rendering this test case
David Kilzer (:ddkilzer)
Comment 5 2006-02-13 14:14:45 PST
This bug may be related to (or the root cause of) Bug 6926 and Bug 7237.
Alexey Proskuryakov
Comment 6 2006-05-18 14:06:27 PDT
*** Bug 8976 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 7 2006-06-04 10:37:52 PDT
*** Bug 9277 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
Comment 8 2006-07-03 09:43:07 PDT
Another manifestation of this caused me several hours of headaches on our site: <div id="test"> <iframe id="iframe" name="iframe" src=""></iframe> <form target="iframe"> <input type="submit" onclick="document.getElementById('test').style.display='none'"/> </form> </div>
Eric Seidel (no email)
Comment 9 2006-07-03 11:02:56 PDT
The effect of the above code, is that instead of submitting the form in the iframe, the user is assaulted by a new window! :(
Adele Peterson
Comment 10 2006-07-03 17:07:46 PDT
we don't actually create Frame objects for frame nodes that don't have renderers. We probably need to start doing that so frame's with display:none get appended to the FrameTree. Or we could find another way to use the FrameArray in JS without using the FrameTree. I'm not entirely sure how well that would work though.
Anders Carlsson
Comment 11 2006-07-05 11:28:18 PDT
Created attachment 9210 [details] Patch
Anders Carlsson
Comment 12 2006-07-05 12:15:53 PDT
Comment on attachment 9210 [details] Patch This introduces crashes
Anders Carlsson
Comment 13 2006-07-05 13:18:52 PDT
Created attachment 9215 [details] Better patch
Anders Carlsson
Comment 14 2006-07-05 13:32:17 PDT
Created attachment 9216 [details] Patch without tabs
Maciej Stachowiak
Comment 15 2006-07-05 13:57:05 PDT
Comment on attachment 9216 [details] Patch without tabs r=me
Anders Carlsson
Comment 16 2006-07-05 14:24:37 PDT
Committed in r15170
David Wheeler
Comment 17 2006-07-05 14:34:57 PDT
Sweet, thanks for the fix. When should we look for a Safari release with this fix?
David Kilzer (:ddkilzer)
Comment 18 2006-07-05 16:13:58 PDT
(In reply to comment #17) > Sweet, thanks for the fix. When should we look for a Safari release with this > fix? Apple doesn't comment on unreleased software, but if I had to guess, it would be Mac OS X 10.5 (Leopard). Note that I am not an Apple engineer, nor am I employed by Apple.
David Wheeler
Comment 19 2006-07-05 16:16:25 PDT
Yeah, that was my guess, too. Pity if true.
Note You need to log in before you can comment on or make changes to this bug.