Bug 107236

Summary: display:none iframes should not build rendering trees
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Layout and RenderingAssignee: Eric Seidel (no email) <eric>
Status: NEW    
Severity: Normal CC: ap, buildbot, cdumez, dglazkov, dholbert, esprehn+autocc, esprehn, kling, koivisto, laszlo.gombos, mkwst, ojan.autocc, rniwa, simon.fraser, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=17828
https://bugs.webkit.org/show_bug.cgi?id=84684
https://bugs.webkit.org/show_bug.cgi?id=6926
https://bugs.webkit.org/show_bug.cgi?id=136890
https://bugs.webkit.org/show_bug.cgi?id=261741
Bug Depends on:    
Bug Blocks: 111200    
Attachments:
Description Flags
Speculative patch, I"m not sure this is the right code to change.
none
For the EWS buildbot: commit-queue-

Eric Seidel (no email)
Reported 2013-01-18 00:11:51 PST
display:none iframes should not build rendering trees
Attachments
Speculative patch, I"m not sure this is the right code to change. (2.11 KB, patch)
2013-01-18 00:13 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
For the EWS (2.36 KB, patch)
2013-03-01 15:12 PST, Eric Seidel (no email) 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2013-01-18 00:13:39 PST
Created attachment 183391 [details] Speculative patch, I"m not sure this is the right code to change.
Eric Seidel (no email)
Comment 2 2013-01-18 00:16:03 PST
I believe this is a regression, but I don't have proof or a timeframe. I just remember that when I wrote the html-parser benchmark 2 years ago it didn't build rendering trees. :) It's possible that the problem is that the HTML parser is using "attach" instead of "lazyAttach" and that lazyAttach would do the right thing as well.
Eric Seidel (no email)
Comment 3 2013-01-18 00:45:45 PST
This may affect whether we issue loads for sub-frames in the display:none iframe. I'm not sure if we have testing coverage for that or not. (I've not yet run the tests with this patch.)
Build Bot
Comment 4 2013-01-18 01:06:32 PST
Comment on attachment 183391 [details] Speculative patch, I"m not sure this is the right code to change. Attachment 183391 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15942428 New failing tests: css2.1/20110323/absolute-replaced-height-007.htm compositing/framesets/composited-frame-alignment.html css2.1/20110323/absolute-replaced-height-025.htm css2.1/20110323/float-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-014.htm css2.1/20110323/block-replaced-height-005.htm css2.1/20110323/block-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-011.htm css2.1/20110323/float-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-035.htm css2.1/20110323/absolute-replaced-height-032.htm css2.1/20110323/absolute-replaced-height-018.htm css2.1/20110323/absolute-replaced-height-028.htm css2.1/20110323/float-replaced-height-004.htm css2.1/20110323/absolute-replaced-height-033.htm css2.1/20110323/absolute-replaced-height-012.htm canvas/philip/tests/2d.text.draw.fontface.notinpage.html compositing/iframes/invisible-nested-iframe-show.html css2.1/20110323/absolute-replaced-height-004.htm css2.1/20110323/inline-block-replaced-height-004.htm css2.1/20110323/absolute-replaced-height-019.htm css2.1/20110323/absolute-replaced-height-026.htm css2.1/20110323/absolute-replaced-height-021.htm css2.1/20110323/block-replaced-height-004.htm
Build Bot
Comment 5 2013-01-18 01:24:18 PST
Comment on attachment 183391 [details] Speculative patch, I"m not sure this is the right code to change. Attachment 183391 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15946311 New failing tests: css2.1/20110323/absolute-replaced-height-007.htm compositing/framesets/composited-frame-alignment.html css2.1/20110323/absolute-replaced-height-025.htm css2.1/20110323/float-replaced-height-005.htm css2.1/20110323/inline-block-replaced-height-007.htm css2.1/20110323/block-replaced-height-005.htm css2.1/20110323/inline-replaced-height-004.htm css2.1/20110323/block-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-011.htm css2.1/20110323/inline-block-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-035.htm css2.1/20110323/inline-replaced-height-005.htm accessibility/loading-iframe-updates-axtree.html css2.1/20110323/absolute-replaced-height-032.htm css2.1/20110323/inline-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-014.htm css2.1/20110323/absolute-replaced-height-018.htm css2.1/20110323/absolute-replaced-height-028.htm css2.1/20110323/float-replaced-height-004.htm css2.1/20110323/absolute-replaced-height-033.htm css2.1/20110323/absolute-replaced-height-012.htm css2.1/20110323/float-replaced-height-007.htm compositing/iframes/invisible-nested-iframe-show.html css2.1/20110323/absolute-replaced-height-004.htm css2.1/20110323/inline-block-replaced-height-004.htm css2.1/20110323/absolute-replaced-height-019.htm css2.1/20110323/absolute-replaced-height-026.htm css2.1/20110323/absolute-replaced-height-021.htm css2.1/20110323/block-replaced-height-004.htm
WebKit Review Bot
Comment 6 2013-01-18 01:25:53 PST
Comment on attachment 183391 [details] Speculative patch, I"m not sure this is the right code to change. Attachment 183391 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15949253 New failing tests: css2.1/20110323/absolute-replaced-height-007.htm compositing/framesets/composited-frame-alignment.html css2.1/20110323/absolute-replaced-height-025.htm css2.1/20110323/float-replaced-height-005.htm css2.1/20110323/inline-block-replaced-height-007.htm css2.1/20110323/block-replaced-height-005.htm css2.1/20110323/inline-replaced-height-004.htm css2.1/20110323/block-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-011.htm css2.1/20110323/inline-block-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-035.htm css2.1/20110323/inline-replaced-height-005.htm css2.1/20110323/absolute-replaced-height-032.htm css2.1/20110323/inline-replaced-height-007.htm css2.1/20110323/absolute-replaced-height-014.htm css2.1/20110323/absolute-replaced-height-018.htm css2.1/20110323/absolute-replaced-height-028.htm css2.1/20110323/float-replaced-height-004.htm css2.1/20110323/absolute-replaced-height-033.htm css2.1/20110323/absolute-replaced-height-012.htm css2.1/20110323/float-replaced-height-007.htm compositing/iframes/invisible-nested-iframe-show.html css2.1/20110323/absolute-replaced-height-004.htm css2.1/20110323/inline-block-replaced-height-004.htm http/tests/misc/favicon-as-image.html css2.1/20110323/absolute-replaced-height-019.htm css2.1/20110323/absolute-replaced-height-026.htm css2.1/20110323/absolute-replaced-height-021.htm css2.1/20110323/block-replaced-height-004.htm
Alexey Proskuryakov
Comment 7 2013-01-18 10:37:10 PST
See also: bug 17828, bug 84684.
Adam Barth
Comment 8 2013-01-18 10:47:54 PST
Comment on attachment 183391 [details] Speculative patch, I"m not sure this is the right code to change. Looks like this causes many tests to fail. It's still a useful patch for us to work with locally to make it easier to profile the parser, but it will probably need more work to land.
Eric Seidel (no email)
Comment 9 2013-03-01 15:12:49 PST
Created attachment 191048 [details] For the EWS
Build Bot
Comment 10 2013-03-01 16:12:04 PST
Comment on attachment 191048 [details] For the EWS Attachment 191048 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16803334 New failing tests: fast/parser/double-write-from-closed-iframe.html fast/forms/textarea-metrics.html compositing/iframes/invisible-nested-iframe-show.html
Eric Seidel (no email)
Comment 11 2013-03-01 16:31:47 PST
IMO, this is the right behavioral change, but getting it exactly right is tricky. We have to be careful to notify the inner document when the outer iframe changes display. I'm not exactly sure how that's working now.
WebKit Review Bot
Comment 12 2013-03-01 19:26:44 PST
Comment on attachment 191048 [details] For the EWS Attachment 191048 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16844171 New failing tests: fast/parser/double-write-from-closed-iframe.html platform/chromium/virtual/softwarecompositing/iframes/invisible-nested-iframe-show.html fast/forms/textarea-metrics.html compositing/iframes/invisible-nested-iframe-show.html
Build Bot
Comment 13 2013-03-02 01:32:04 PST
Comment on attachment 191048 [details] For the EWS Attachment 191048 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16855312 New failing tests: fast/parser/double-write-from-closed-iframe.html fast/forms/textarea-metrics.html compositing/iframes/invisible-nested-iframe-show.html
Alexey Proskuryakov
Comment 14 2016-09-26 09:26:20 PDT
*** Bug 162560 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 15 2016-09-26 09:26:55 PDT
See https://github.com/whatwg/html/issues/1813
Note You need to log in before you can comment on or make changes to this bug.