Bug 172266 – REGRESSION (new media controls): backdrops appear with visibility:hidden, showing new media controls when it shouldn't on apple.com

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Layout and Rendering

[Antti Koivisto]

-webkit-appearance: media-controls-light-bar-background has problems with visibility.

[Antti Koivisto]

Created attachment 310501 [details]
patch

[Antti Koivisto]

<rdar://problem/32141959>

[Antti Koivisto]

Alternatively or additionally we could simply stop generating layers for fully invisible subtrees. We track this already in the RenderLayer tree.

[Antti Koivisto]

Created attachment 310508 [details]
patch

A better patch that blocks editing for UA shadow trees. The changed to eliminate raw pointers from SearchInputType are also retained.

[Antti Koivisto]

Oops wrong bug. The original patch is for this one.

[Simon Fraser (smfr)]

Comment on attachment 310501 [details]
patch

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

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1640
> +    bool needBackdropLayerType = (customAppearance() == LightBackdropAppearance || customAppearance() == DarkBackdropAppearance) && m_contentsVisible;

It's a bit weird to change the layer type based on visibility. What's the actual problem here; does CA still do backdrop stuff on layers with the hidden attribute, or do we fail to set the hidden attribute on some of the layers we create?

> LayoutTests/compositing/visibility/visibility-custom-backdrop-appearance.html:74
> +    toggle.offsetWidth;

Why do you need to force layout?

> LayoutTests/compositing/visibility/visibility-custom-backdrop-appearance.html:75
> +    toggle.classList.toggle("hidden");

You could just put a class on document.body and rewrite the selectors.

[Antti Koivisto]

> It's a bit weird to change the layer type based on visibility. What's the
> actual problem here; does CA still do backdrop stuff on layers with the
> hidden attribute, or do we fail to set the hidden attribute on some of the
> layers we create?

Custom backdrop layer replaces m_layer and for m_layer we just null out the content (and call setContentsHidden which doesn't do much) instead of calling setHidden. Visible stuff on backdrop layer is not content so we fail to hide it.

The logic here is that we don't need backdrop layer at all when it is not visible.

> > LayoutTests/compositing/visibility/visibility-custom-backdrop-appearance.html:74
> > +    toggle.offsetWidth;
> 
> Why do you need to force layout?

I wanted to ensure that there is a mutation in the layer tree. I'm not actually sure if this is sufficient for that.

[Antti Koivisto]

We could alternatively call setHidden in updateContentsVisibility when isCustomBackdropLayerType(m_layer->layerType()). That is more lines of code and I'm not sure if it is any better.

[Antti Koivisto]

https://bugs.webkit.org/show_bug.cgi?id=172499 takes care of this.