Bug 125565 – Frame's composited content is visible when the frame has visibility: hidden.

• Component: CSS
• Status: RESOLVED FIXED

[j5726]

If a page contains an iframe, and there're some elements in the iframe been set as -webkit-transform: translate3d(0px, 0px, 0) . 

When set the iframe as visibility: hidden , It can't be hidden normally. 

visibility:hidden is invalid on an iframe with elements been set as -webkit-transform: translate3d

[j5726]

Created attachment 218950 [details]
the iframe visibility:hidden test case

If a page contains an iframe, and there're some elements in the iframe been set as -webkit-transform: translate3d(0px, 0px, 0) . 

When set the iframe as visibility: hidden , It can't be hidden normally.

[Manish Gurnaney]

Created attachment 225450 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 225450 [details]
Patch

Very bizarre patch. Why non-debug only? Not painting isn't the correct solution either; the actual bug here is that the WinCairo compositing code doesn't respect visibility:hidden for iframes (although maybe that's an issue with other platforms). Does this work on Mac, or iOS?

[Manish Gurnaney]

Hi Simon,
   I have checked the issue on Mac, Linux and Windows. Issues is getting reproducible in Mac(Safari and Chrome), Linux(Efl Port), Windows (WinCG, Safari and Chrome). So the issues is not specific to WinCG.
   Also I have observed that issue is happening in case of Webkit transform property that too when only 3d properties are given or when RenderLayerBacking is getting created. When we paint any element we do check for the Visibility of the element.
   Hence I am checking here in RenderLayerBacking itself.
--Please Suggest

[Manish Gurnaney]

Created attachment 225841 [details]
Patch

[Manish Gurnaney]

Hi Simon, 
        Can you please review the updated patch.
Regards,
Manish R Gurnaney

[zalan]

Comment on attachment 225841 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225841&action=review

> Source/WebCore/rendering/RenderLayerBacking.cpp:2204
> +        return;

It doesnt make much sense to create compositing for something that we never display. I think we should make a decision earlier so that an iframe with visibility: hidden would not be creating a compositing layer. If we manage to not create this compositing layer, existing code will probably take care of the not-painting part.

[Manish Gurnaney]

Created attachment 227005 [details]
Patch

[Build Bot]

Comment on attachment 227005 [details]
Patch

Attachment 227005 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6589632393248768

New failing tests:
fast/events/mouseover-mouseout2.html
compositing/iframes/iframe-visibility-hidden.html
compositing/visibility/hidden-iframe.html

[Build Bot]

Created attachment 227009 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227005 [details]
Patch

Attachment 227005 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6597531911847936

New failing tests:
fast/events/mouseover-mouseout2.html
compositing/iframes/iframe-visibility-hidden.html
compositing/visibility/hidden-iframe.html

[Build Bot]

Created attachment 227012 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227005 [details]
Patch

Attachment 227005 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5141360831102976

New failing tests:
fast/events/mouseover-mouseout2.html
compositing/iframes/iframe-visibility-hidden.html
compositing/visibility/hidden-iframe.html

[Build Bot]

Created attachment 227014 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Manish Gurnaney]

Created attachment 227030 [details]
Patch

[Build Bot]

Comment on attachment 227030 [details]
Patch

Attachment 227030 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6495959156523008

New failing tests:
compositing/iframes/iframe-visibility-hidden.html

[Build Bot]

Created attachment 227035 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227030 [details]
Patch

Attachment 227030 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5211600726261760

New failing tests:
media/W3C/audio/canPlayType/canPlayType_application_octet_stream.html
compositing/iframes/iframe-visibility-hidden.html

[Build Bot]

Created attachment 227040 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Manish Gurnaney]

Created attachment 227042 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 227042 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227042&action=review

> Source/WebCore/rendering/RenderIFrame.cpp:66
>  bool RenderIFrame::requiresLayer() const
>  {
> -    return RenderFrameBase::requiresLayer() || style().resize() != RESIZE_NONE;
> +    return (style().visibility() == VISIBLE || style().hasOutOfFlowPosition())
> +        && (RenderFrameBase::requiresLayer() || style().resize() != RESIZE_NONE);
>  }

This is the wrong place. This is about RenderLayer creation, not compositing (GraphicsLayer) creation.

RenderLayerCompositor::requiresCompositingForFrame() is the one you should change.

[Manish Gurnaney]

Created attachment 227156 [details]
Patch

[Build Bot]

Comment on attachment 227156 [details]
Patch

Attachment 227156 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5917214460870656

New failing tests:
compositing/iframes/iframe-visibility-hidden.html

[Build Bot]

Created attachment 227164 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227156 [details]
Patch

Attachment 227156 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5733958943768576

New failing tests:
compositing/iframes/iframe-visibility-hidden.html

[Build Bot]

Created attachment 227166 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Manish Gurnaney]

Created attachment 227403 [details]
Patch

[zalan]

Comment on attachment 227403 [details]
Patch

The code change looks good, however I'd create a ref test instead testing maybe even against a non-layer case. I'd also add a test case to check if dynamically removing the hidden property will bring the compositing content back.

[Simon Fraser (smfr)]

Comment on attachment 227403 [details]
Patch

r- to add the tests that Zalan suggets.

[Manish Gurnaney]

Created attachment 228594 [details]
Patch

[Build Bot]

Comment on attachment 228594 [details]
Patch

Attachment 228594 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5326209713963008

New failing tests:
compositing/iframes/iframe-visibility-change-dynamically.html

[Build Bot]

Created attachment 228606 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Manish Gurnaney]

Created attachment 234746 [details]
Patch

[Build Bot]

Comment on attachment 234746 [details]
Patch

Attachment 234746 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6095123725156352

New failing tests:
compositing/iframes/iframe-visibility-change-dynamically.html

[Build Bot]

Created attachment 234749 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 234746 [details]
Patch

Attachment 234746 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6329632160743424

New failing tests:
compositing/iframes/iframe-visibility-change-dynamically.html

[Build Bot]

Created attachment 234752 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Darin Adler]

Comment on attachment 234746 [details]
Patch

Change looks OK, but the test did not pass on the EWS bot, so I am concerned.

[Manish Gurnaney]

I have added two test cases in support of my Patch, where one test Cases is getting failed (iframe-visibility-change-dynamically.html).

In this test cases Iframe is visibility is hidden initially, and dynamically we are changing visibility to visible. But this case is getting failed in mac as when visibility is changed, all the elements are rendered except the element having property (-webkit-transform: translate3d(0px, 0px, 0);) due to which test case is getting failed.

I tried to run this test case on Webkit Gtk and efl port in both the cases it is getting passed. It is failing only in mac.

I am not having mac machine with me, hence struggling to get the route cause of this issue. Can you please suggest what can be done in this case?

[Tim Horton]

*** Bug 134774 has been marked as a duplicate of this bug. ***

[Manish Gurnaney]

Hi Darin/Zalan, 
              Can you please suggest what can be done in this case ?
Manish

[zalan]

Created attachment 311239 [details]
Test reduction

[zalan]

rdar://problem/32196849

[zalan]

anything that's a RenderWidget.

[zalan]

Created attachment 311313 [details]
Patch

[Build Bot]

Comment on attachment 311313 [details]
Patch

Attachment 311313 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3818142

New failing tests:
compositing/visibility/iframe-visibility-hidden.html

[Build Bot]

Created attachment 311321 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[zalan]

Created attachment 311325 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 311325 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311325&action=review

> LayoutTests/compositing/visibility/iframe-visibility-hidden.html:14
> +    document.getElementById("divToVisible").style.visibility = "visible";

But divToVisible (which is actually an iframe) is already visible?

> LayoutTests/compositing/visibility/iframe-visibility-hidden.html:44
> +<iframe frameborder=no id=divToVisible onload="makeDivVisible()" width="80" height="80" style="visibility: visible"

Should this start hidden?

[zalan]

Created attachment 311328 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 311328 [details]
Patch

Clearing flags on attachment: 311328

Committed r217472: <http://trac.webkit.org/changeset/217472>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.