| Summary: | display:none iframes should not build rendering trees | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
| Component: | Layout and Rendering | Assignee: | 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
Eric Seidel (no email)
2013-01-18 00:11:51 PST
Created attachment 183391 [details]
Speculative patch, I"m not sure this is the right code to change.
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. 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 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 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 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 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.
Created attachment 191048 [details]
For the EWS
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 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 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 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 *** Bug 162560 has been marked as a duplicate of this bug. *** |