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, 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
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-

Description Eric Seidel (no email) 2013-01-18 00:11:51 PST
display:none iframes should not build rendering trees
Comment 1 Eric Seidel (no email) 2013-01-18 00:13:39 PST
Created attachment 183391 [details]
Speculative patch, I"m not sure this is the right code to change.
Comment 2 Eric Seidel (no email) 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.
Comment 3 Eric Seidel (no email) 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.)
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 WebKit Review Bot 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
Comment 7 Alexey Proskuryakov 2013-01-18 10:37:10 PST
See also: bug 17828, bug 84684.
Comment 8 Adam Barth 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.
Comment 9 Eric Seidel (no email) 2013-03-01 15:12:49 PST
Created attachment 191048 [details]
For the EWS
Comment 10 Build Bot 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
Comment 11 Eric Seidel (no email) 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.
Comment 12 WebKit Review Bot 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
Comment 13 Build Bot 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
Comment 14 Alexey Proskuryakov 2016-09-26 09:26:20 PDT
*** Bug 162560 has been marked as a duplicate of this bug. ***
Comment 15 Alexey Proskuryakov 2016-09-26 09:26:55 PDT
See https://github.com/whatwg/html/issues/1813