RESOLVED FIXED 46223
Can't put flash object over an iframe shim in Chromium
https://bugs.webkit.org/show_bug.cgi?id=46223
Summary Can't put flash object over an iframe shim in Chromium
Ananta Iyengar
Reported 2010-09-21 15:26:55 PDT
This bug affects Chromium only. The Chrome bug is here http://code.google.com/p/chromium/issues/detail?id=54200 Steps 1. Have an iframe element with a page loaded within. 2. Use script, etc. to place a flash object from the parent page on top of the iframe object. 3. Even if you set the flash object's z-index higher than the iframe's, the flash object will be displayed under the iframe Here is a site which has this problem. http://mixla.org The problem appears to affect the Windows port of Chromium only.
Attachments
Initial patch for this issue (2.10 KB, patch)
2010-09-21 18:37 PDT, Ananta Iyengar
no flags
Patch with updated comments (2.35 KB, patch)
2010-09-22 07:10 PDT, Ananta Iyengar
no flags
New patch with the iframe-shims.html Layout test updated to test this case. (5.26 KB, patch)
2010-09-25 10:20 PDT, Ananta Iyengar
no flags
Updated patch with the iframe-shims.html layout test updated to test for this case. (5.26 KB, patch)
2010-09-25 10:24 PDT, Ananta Iyengar
no flags
Updated patch with formatting errors fixed (5.27 KB, text/plain)
2010-09-25 10:27 PDT, Ananta Iyengar
no flags
Updated patch with formatting errors fixed (5.27 KB, patch)
2010-09-25 10:28 PDT, Ananta Iyengar
no flags
Fixed the formatting in the layout test (5.32 KB, patch)
2010-09-25 10:31 PDT, Ananta Iyengar
no flags
Hopefully the last of the formatting fix. (4.49 KB, patch)
2010-09-25 10:34 PDT, Ananta Iyengar
no flags
Updated patch with the iframe-shims.html added to skipped list for mac platforms. (9.47 KB, patch)
2010-09-27 19:19 PDT, Ananta Iyengar
levin: review-
Updated patch with the iframe-shims.html skipped for mac, win and qt. (8.63 KB, patch)
2010-09-28 11:11 PDT, Ananta Iyengar
no flags
Updated patch with style issues and svn conflicts fixed. (8.76 KB, patch)
2010-09-28 12:21 PDT, Ananta Iyengar
no flags
Ananta Iyengar
Comment 1 2010-09-21 18:37:22 PDT
Created attachment 68321 [details] Initial patch for this issue
Ananta Iyengar
Comment 2 2010-09-21 18:39:02 PDT
I discussed this issue with Thatcher Ulrich from Google who originally wrote this code for Chromium and John Newlin from Google who rewrote parts of the iframe shim code to fix a layering bug. We agreed that the proposed fix is the right fix in the short term. Longer term we should look into Firefox's implementation which seems to do the right thing even if there is no iframe shim. I tested this fix with the test case mentioned in the bug and the test cases on the site maintained by Thatecher Ulrich. They all work fine. The plugin layout test iframe-shims.html also works fine. Please take a look at the proposed patch. It is relatively low risk as the only consumer of this code is Chromium.
Darin Fisher (:fishd, Google)
Comment 3 2010-09-22 00:28:57 PDT
Comment on attachment 68321 [details] Initial patch for this issue View in context: https://bugs.webkit.org/attachment.cgi?id=68321&action=review > WebKit/chromium/src/WebPluginContainerImpl.cpp:545 > + // it stacks behind the iframe. it seems like the comment needs to be updated. > WebKit/chromium/src/WebPluginContainerImpl.cpp:547 > + const RenderObject* pluginRenderObject = pluginZstack[0]; it is not obvious to me why you always check the 0'th elements of pluginZstack and iframeZstack here.
Ananta Iyengar
Comment 4 2010-09-22 07:10:03 PDT
Created attachment 68368 [details] Patch with updated comments
Ananta Iyengar
Comment 5 2010-09-22 07:11:23 PDT
Hi Darin Thanks for the review. I updated the earlier comment and added a note indicating why we check the 0'th elements of the RenderObject arrays. Thanks Ananta
Darin Fisher (:fishd, Google)
Comment 6 2010-09-24 09:01:29 PDT
Comment on attachment 68368 [details] Patch with updated comments R=me, but is it possible to write a layout test for this extending or building upon the layout test that already exists for iframe shims?
Ananta Iyengar
Comment 7 2010-09-25 10:20:04 PDT
Created attachment 68829 [details] New patch with the iframe-shims.html Layout test updated to test this case.
WebKit Review Bot
Comment 8 2010-09-25 10:24:49 PDT
Attachment 68829 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/ChangeLog:5: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Ananta Iyengar
Comment 9 2010-09-25 10:24:57 PDT
Created attachment 68830 [details] Updated patch with the iframe-shims.html layout test updated to test for this case.
Ananta Iyengar
Comment 10 2010-09-25 10:27:42 PDT
Created attachment 68831 [details] Updated patch with formatting errors fixed
Ananta Iyengar
Comment 11 2010-09-25 10:28:47 PDT
Created attachment 68832 [details] Updated patch with formatting errors fixed
Ananta Iyengar
Comment 12 2010-09-25 10:31:52 PDT
Created attachment 68833 [details] Fixed the formatting in the layout test
Ananta Iyengar
Comment 13 2010-09-25 10:34:55 PDT
Created attachment 68834 [details] Hopefully the last of the formatting fix.
Darin Fisher (:fishd, Google)
Comment 14 2010-09-27 09:28:34 PDT
Comment on attachment 68834 [details] Hopefully the last of the formatting fix. Thanks for the test! R=me
WebKit Commit Bot
Comment 15 2010-09-27 09:48:33 PDT
Comment on attachment 68834 [details] Hopefully the last of the formatting fix. Rejecting patch 68834 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']" exit_code: 2 Last 500 characters of output: r....ok All tests successful. Files=14, Tests=304, 1 wallclock secs ( 0.72 cusr + 0.16 csys = 0.88 CPU) Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Projects/CommitQueue/LayoutTests Testing 21378 test cases. plugins/iframe-shims.html -> failed Exiting early after 1 failures. 17854 tests run. 317.92s total testing time 17853 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 31 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/4132012
Ananta Iyengar
Comment 16 2010-09-27 19:19:26 PDT
Created attachment 69012 [details] Updated patch with the iframe-shims.html added to skipped list for mac platforms.
Eric Seidel (no email)
Comment 17 2010-09-28 03:18:45 PDT
Comment on attachment 68368 [details] Patch with updated comments Cleared Darin Fisher's review+ from obsolete attachment 68368 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Eric Seidel (no email)
Comment 18 2010-09-28 03:18:49 PDT
Comment on attachment 68834 [details] Hopefully the last of the formatting fix. Cleared Darin Fisher's review+ from obsolete attachment 68834 [details] so that this bug does not appear in http://webkit.org/pending-commit.
David Levin
Comment 19 2010-09-28 10:29:57 PDT
Comment on attachment 69012 [details] Updated patch with the iframe-shims.html added to skipped list for mac platforms. View in context: https://bugs.webkit.org/attachment.cgi?id=69012&action=review > LayoutTests/platform/mac-leopard/Skipped:106 > +plugins/iframe-shims.html LayoutTests/platform/mac/Skipped should be sufficent. > LayoutTests/platform/mac-snowleopard/Skipped:143 > +plugins/iframe-shims.html LayoutTests/platform/mac/Skipped should be sufficent. > LayoutTests/platform/mac-tiger/Skipped:212 > +plugins/iframe-shims.html LayoutTests/platform/mac/Skipped should be sufficent. > LayoutTests/platform/mac/Skipped:326 > +plugins/iframe-shims.html What about other platforms like LayoutTests/platform/win/Skipped?
Ananta Iyengar
Comment 20 2010-09-28 11:11:06 PDT
Created attachment 69073 [details] Updated patch with the iframe-shims.html skipped for mac, win and qt.
Ananta Iyengar
Comment 21 2010-09-28 12:21:01 PDT
Created attachment 69085 [details] Updated patch with style issues and svn conflicts fixed.
WebKit Commit Bot
Comment 22 2010-09-29 10:15:14 PDT
Comment on attachment 69085 [details] Updated patch with style issues and svn conflicts fixed. Clearing flags on attachment: 69085 Committed r68656: <http://trac.webkit.org/changeset/68656>
WebKit Commit Bot
Comment 23 2010-09-29 10:15:20 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.