Can't put flash object over an iframe shim in Chromium

• Component: Plug-ins
• Status: RESOLVED FIXED

[Ananta Iyengar]

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.

[Ananta Iyengar]

Created attachment 68321 [details]
Initial patch for this issue

[Ananta Iyengar]

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

Created attachment 68368 [details]
Patch with updated comments

[Ananta Iyengar]

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

Created attachment 68829 [details]
New patch with the iframe-shims.html Layout test updated to test this case.

[WebKit Review Bot]

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]

Created attachment 68830 [details]
Updated patch with the iframe-shims.html layout test updated to test for this case.

[Ananta Iyengar]

Created attachment 68831 [details]
Updated patch with formatting errors fixed

[Ananta Iyengar]

Created attachment 68832 [details]
Updated patch with formatting errors fixed

[Ananta Iyengar]

Created attachment 68833 [details]
Fixed the formatting in the layout test

[Ananta Iyengar]

Created attachment 68834 [details]
Hopefully the last of the formatting fix.

[Darin Fisher (:fishd, Google)]

Comment on attachment 68834 [details]
Hopefully the last of the formatting fix.

Thanks for the test!  R=me

[WebKit Commit Bot]

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]

Created attachment 69012 [details]
Updated patch with the iframe-shims.html added to skipped list for mac platforms.

[Eric Seidel (no email)]

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

Created attachment 69073 [details]
Updated patch with the iframe-shims.html skipped for mac, win and qt.

[Ananta Iyengar]

Created attachment 69085 [details]
Updated patch with style issues and svn conflicts fixed.

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.