Bug 253706 - RenderLayer::hasVisibleContent() incorrect when layer removed
Summary: RenderLayer::hasVisibleContent() incorrect when layer removed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ahmad Saleem
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2023-03-10 08:08 PST by Ahmad Saleem
Modified: 2023-03-29 12:34 PDT (History)
6 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-03-10 08:08:24 PST 
Hi Team,

While going through Blink's commit, I came across another failing test case:

Test case - https://jsfiddle.net/b7kwpvx8/1/

^ Safari Technology Preview 164 show 'Red' while Chrome Canary 113 and Firefox Nightly 112 show 'Green'.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/c5587982b1ed1ec62452f5d2c93c0f385a3941a1

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderLayer.cpp#459

Below this WebKit source, in local testing when I add following:

if (renderer().style().visibility() != Visibility::Visible)
        dirtyVisibleContentStatus();

_________

It fixes this bug and show 'Green' similar to other browsers.

Appreciate if someone can share input and I am happy to do PR.

Thanks!
Comment 1 Radar WebKit Bug Importer 2023-03-17 09:09:15 PDT 
<rdar://problem/106860749>
Comment 2 Karl Dubost 2023-03-26 20:59:03 PDT 
That looks like a low hanging fruit.
Comment 3 zalan 2023-03-26 21:15:41 PDT 
Indeed.
diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index db8ca3df7421..432a841c61e8 100644
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -475,6 +475,8 @@ void RenderLayer::removeChild(RenderLayer& oldChild)
     if (oldChild.hasBlendMode() || (oldChild.hasNotIsolatedBlendingDescendants() && !oldChild.isolatesBlending()))
         dirtyAncestorChainHasBlendingDescendants();
 #endif
+    if (renderer().style().visibility() != Visibility::Visible)
+        dirtyVisibleContentStatus();
 }
 
 void RenderLayer::dirtyPaintOrderListsOnChildChange(RenderLayer& child)

shows green.
Comment 4 Ahmad Saleem 2023-03-27 07:41:04 PDT 
(In reply to zalan from comment #3)
> Indeed.
> diff --git a/Source/WebCore/rendering/RenderLayer.cpp
> b/Source/WebCore/rendering/RenderLayer.cpp
> index db8ca3df7421..432a841c61e8 100644
> --- a/Source/WebCore/rendering/RenderLayer.cpp
> +++ b/Source/WebCore/rendering/RenderLayer.cpp
> @@ -475,6 +475,8 @@ void RenderLayer::removeChild(RenderLayer& oldChild)
>      if (oldChild.hasBlendMode() ||
> (oldChild.hasNotIsolatedBlendingDescendants() &&
> !oldChild.isolatesBlending()))
>          dirtyAncestorChainHasBlendingDescendants();
>  #endif
> +    if (renderer().style().visibility() != Visibility::Visible)
> +        dirtyVisibleContentStatus();
>  }
>  
>  void RenderLayer::dirtyPaintOrderListsOnChildChange(RenderLayer& child)
> 
> shows green.

Running through EWS - https://github.com/WebKit/WebKit/pull/12005

Let's see.
Comment 5 Ahmad Saleem 2023-03-29 10:53:48 PDT 
PR - https://github.com/WebKit/WebKit/pull/12005
Comment 6 EWS 2023-03-29 12:34:13 PDT 
Committed 262284@main (059e2bb75dc3): <https://commits.webkit.org/262284@main>

Reviewed commits have been landed. Closing PR #12005 and removing active labels.