Bug 3778

This is reproducable in current ToT

it's the left side of the page here, but i can reproduce it :). confirming.

Created attachment 2794 [details] Reduction of the problem Text, images and the movies control bar seem to work correctly, it's just the movie window itself

<rdar://problem/4338918>

Created attachment 7930 [details] Related Reduction This is a very similar case where the QuickTime movie overlaps a fixed area that it should scroll under. It appears that QuickTime fails to properly inherit the z-index.

the same thing happens with flash movies. when they are positioned in a div with scrolling overflow the browser continues to render the flash element once it's outside of the div and should not be visible. http://dev.exit3a.com/main.html

Created attachment 101095 [details] Patch

A fixed position element should occlude a plugin if it has a higher index. Though firefox and opera both have some odd rules for this. The fixed position element will only occlude the plugin, regardless of z index, if: - It has a non-transparent background - It's not a form control, e.g. an input element I cannot find any source to explain this behaviour so instead of mimicking it I've implemented what seems the correct thing to do in the absence of a spec - allow fixed position elements to always occlude a plugin if they have a higher z index. If the z-index is equal, the plugin will also get occluded - based on the fact that the use-case of fixed position elements is to keep them always in view.

(In reply to comment #7) > Created an attachment (id=101095) [details] > Patch Note that IFrameShimSupport needs to be changed to something like PluginOcclusions - but I did not rename in this patch as it would obscure the changes to the file in the patch.

Comment on attachment 101095 [details] Patch The code should be walking the RenderLayer z-order tree to determine stacking, rather than trying to do it itself.

Created attachment 102961 [details] Patch

(In reply to comment #10) > (From update of attachment 101095 [details]) > The code should be walking the RenderLayer z-order tree to determine stacking, rather than trying to do it itself. Hi Simon, I've taken your advice and use the RenderLayer to implement occluding plugins by all elements, be they iframes or otherwise. It results in substantially less code. Could you take another look when you get the chance? Darin, since this rewrites the iframe shim support, could you also take a look? Thanks, Robert

Comment on attachment 102961 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102961&action=review Is the RenderLayer tree guaranteed to be current when getPluginOcclusions() is called? Are those plugin methods called during layout? > Source/WebCore/plugins/IFrameShimSupport.cpp:65 > + layer->dirtyZOrderLists(); Why are you dirtying the z-order lists here? > Source/WebCore/plugins/IFrameShimSupport.cpp:79 > + if (layer == element->renderer()->enclosingLayer()) { You should put element->renderer()->enclosingLayer() in a local variable. It's called to get the initial 'sc' too. > Source/WebCore/plugins/IFrameShimSupport.cpp:100 > + for (RenderLayer* child = layer->firstChild(); child; child = child->nextSibling()) > + if (child->renderer()->hasBackground()) > + appendOcclusion(toRenderBox(child->renderer()), occlusions); Why is this looking at layer children, not renderer children?

(In reply to comment #13) > (From update of attachment 102961 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102961&action=review > > Is the RenderLayer tree guaranteed to be current when getPluginOcclusions() is called? Are those plugin methods called during layout? getPluginOcclusions() is called during layout, but I'm not sure that the RenderLayer tree is current when it is called. How can I ensure that it is apart from calling updateZOrderLists()? > > > Source/WebCore/plugins/IFrameShimSupport.cpp:65 > > + layer->dirtyZOrderLists(); > > Why are you dirtying the z-order lists here? A leftover from when I was getting the hang of the zorder list. > > Source/WebCore/plugins/IFrameShimSupport.cpp:100 > > + for (RenderLayer* child = layer->firstChild(); child; child = child->nextSibling()) > > + if (child->renderer()->hasBackground()) > > + appendOcclusion(toRenderBox(child->renderer()), occlusions); > > Why is this looking at layer children, not renderer children? Simply because iterating through the layers was the first thing that occurred to me and it worked. Why is using the renderer children more correct?

(In reply to comment #14) > (In reply to comment #13) > > (From update of attachment 102961 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=102961&action=review > > > > Is the RenderLayer tree guaranteed to be current when getPluginOcclusions() is called? Are those plugin methods called during layout? > > getPluginOcclusions() is called during layout... This is incorrect, it's called when the plugin is getting painted, so at that point layout is meant to be complete. I've updated the patch per your other comments.

When revising the patch I noticed a problem with the layout test, and in an effort to fix it discovered that the assumptions behind the current plugins/iframe-shims.html and my new test are wrong. The current iframe-shims.html test passes even when no attempt is made to occlude the plugin (by commenting out the call to getPluginOcclusions()). Both tests work fine manually (using QtTestBrowser for example), but the behaviour of the hit testing when run from DumpRenderTree does not depend at all on whether the plugins occlude the elements above/below them or not. It's not due to using flash plugins in the tests as the same problem exhibits with the test netscape plugin. I suspect the problem is the use of windowed plugins run from DumpRenderTree without displaying the webview. So it's possible to fix this bug, but currently not possible to write a test for it, and the existing plugins/iframe-shims.html test will continue to pass even if a regression happens. Any suggestions how to proceed? I'm stuck.

Can we make TestNetscapePlugin not windowed? Or have a not-windowed plugin? We certainly use flash in other layout tests, seems we could use it here too.

Comment on attachment 102961 [details] Patch Based on smfr's comments, seems this needs at least one more round.

Any progress on this bug?

(In reply to comment #19) > Any progress on this bug? I think the code-change is good - but it's extremely hard to create a valid test. The current z-pos/plugin testing coverage ( plugins/iframe-shims.html) is not valid either and will happily pass even plugin occlusions regress. I would love a manual-test pass on this one as the effort/investigation required to get the testing working (some kind of fu required in TestNetscapePlugin) has eluded me. It's not windowed/not-windowed - it is more to do with the need to get a painted background going in TestNetscapePlugin so that the hit-testing can work properly.

Created attachment 175605 [details] rebase prev patch to HEAD

I updated Robert's patch for a current checkout. Two changes were needed: - In getPluginOcclusions, replaced layer->dirtyZOrderLists(); layer->updateZOrderLists(); // this is now private with layer->updateLayerListsIfNeeded(); - PluginViewQt.cpp cannot be patched, as the getPluginOcclusions() call was removed in http://trac.webkit.org/changeset/124879 It's too bad a layout test is difficult due to hit testing for plugins run in DumpRenderTree. I guess just a pixel test is also hard?

Comment on attachment 175605 [details] rebase prev patch to HEAD Attachment 175605 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14959291 New failing tests: plugins/fixed-position-object-occludes-plugin.html

Comment on attachment 175605 [details] rebase prev patch to HEAD Ist this change Chromium-only? If so, the bug title should be prepended with [Chromium]

This is an ancient 4-digit bug. :) Robert was reporting that this occurs for him on Chromium Linux with NPAPI flash. But I'm not able to reproduce this bug on either Safari or Chrome (with built-in PPAPI flash) on my Mac.

This original reporter used the QuickTime plugin in Safari.

This bug was reported against Mac, and this has been working correctly on Mac since at least Snow Leopard.