Bug 228124 - If the color scheme of an iframe differs from embedding document iframe gets an opaque canvas bg appropriate to its color scheme
Summary: If the color scheme of an iframe differs from embedding document iframe gets ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 14
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-20 13:56 PDT by Adam Argyle
Modified: 2021-09-03 13:33 PDT (History)
12 users (show)

See Also:


Attachments
Provided demo url loaded in Safari, code is on top, and an empty demo are on bottom (307.50 KB, image/png)
2021-07-20 13:56 PDT, Adam Argyle
no flags Details
`html { background-color: Canvas; }` fix (250.51 KB, image/png)
2021-07-21 14:37 PDT, Adam Argyle
no flags Details
Patch (10.90 KB, patch)
2021-08-31 11:38 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Patch (12.39 KB, patch)
2021-08-31 13:48 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Argyle 2021-07-20 13:56:04 PDT
Created attachment 433900 [details]
Provided demo url loaded in Safari, code is on top, and an empty demo are on bottom

Renders a blank looking (demo in an iframe):
https://codepen.io/argyleink/pen/rNmzGzW

Renders as expected (demo not in an iframe):
https://codepen.io/argyleink/debug/rNmzGzW

Can be mitigated by not using `color-scheme` or adjusting the values used to not include "dark".
Comment 1 Simon Fraser (smfr) 2021-07-20 20:37:20 PDT
We've got white text on white in the <iframe>. codepen styles the iframe background color to be white, and the default text color respects dark mode, so I think this is expected?
Comment 2 Adam Argyle 2021-07-21 14:37:20 PDT
Created attachment 433957 [details]
`html { background-color: Canvas; }` fix

Here I set the background color of the document explicitly (though isn't this what it should be by default?), and then the iframe renders as expected. It seems like the iframe background color is taking precedence when the document clearly has a preference request with color-scheme?
Comment 3 Simon Fraser (smfr) 2021-07-21 14:40:35 PDT
iframe content background is transparent by default, and controlled by the enclosing page's style on the iframe. I don't think the UA can override that?
Comment 5 Simon Fraser (smfr) 2021-07-21 15:38:57 PDT
Nice!
Comment 6 Radar WebKit Bug Importer 2021-07-21 15:39:35 PDT
<rdar://problem/80922070>
Comment 7 Aditya Keerthi 2021-08-31 11:38:15 PDT
Created attachment 436918 [details]
Patch
Comment 8 EWS Watchlist 2021-08-31 11:39:13 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 9 Simon Fraser (smfr) 2021-08-31 12:11:25 PDT
Comment on attachment 436918 [details]
Patch

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

> Source/WebCore/rendering/RenderBoxModelObject.cpp:945
> +        auto& frameView = view().frameView();
> +
> +        isOpaqueRoot = [&] {
> +            if (bgLayer.next() || bgColor.isOpaque())
> +                return true;
> +
> +            auto* ownerElement = document().ownerElement();
> +
> +            // Fill with a base color if we're the root document.
> +            if (!ownerElement)
> +                return !frameView.isTransparent();
> +
> +            if (ownerElement->hasTagName(frameTag))
> +                return true;
> +
> +            // Locate the <body> element using the DOM. This is easier than trying
> +            // to crawl around a render tree with potential :before/:after content and
> +            // anonymous blocks created by inline <body> tags etc. We can locate the <body>
> +            // render object very easily via the DOM.
> +            auto* body = document().bodyOrFrameset();
> +
> +            // SVG documents and XML documents with SVG root nodes are transparent.
> +            if (!body)
> +                return !document().hasSVGRootNode();
> +
> +            // Can't scroll a frameset document anyway.
> +            if (is<HTMLFrameSetElement>(*body))
> +                return true;
> +
> +            auto* frameRenderer = ownerElement->renderer();
> +            if (!frameRenderer)
> +                return false;
> +
> +            // iframes should fill with a base color if the used color scheme of the
> +            // element and the used color scheme of the embedded documentâs root
> +            // element do not match.
> +            if (frameView.useDarkAppearance() != frameRenderer->useDarkAppearance())
> +                return !frameView.isTransparent();
> +
> +            return false;
> +        }();
> +
> +        frameView.setContentIsOpaque(isOpaqueRoot);

Let's pull this root background paint into new function.
Comment 10 Aditya Keerthi 2021-08-31 13:48:19 PDT
Created attachment 436937 [details]
Patch
Comment 11 Aditya Keerthi 2021-08-31 13:49:24 PDT
(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 436918 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=436918&action=review
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:945
> > +        auto& frameView = view().frameView();
> > +
> > +        isOpaqueRoot = [&] {
> > +            if (bgLayer.next() || bgColor.isOpaque())
> > +                return true;
> > +
> > +            auto* ownerElement = document().ownerElement();
> > +
> > +            // Fill with a base color if we're the root document.
> > +            if (!ownerElement)
> > +                return !frameView.isTransparent();
> > +
> > +            if (ownerElement->hasTagName(frameTag))
> > +                return true;
> > +
> > +            // Locate the <body> element using the DOM. This is easier than trying
> > +            // to crawl around a render tree with potential :before/:after content and
> > +            // anonymous blocks created by inline <body> tags etc. We can locate the <body>
> > +            // render object very easily via the DOM.
> > +            auto* body = document().bodyOrFrameset();
> > +
> > +            // SVG documents and XML documents with SVG root nodes are transparent.
> > +            if (!body)
> > +                return !document().hasSVGRootNode();
> > +
> > +            // Can't scroll a frameset document anyway.
> > +            if (is<HTMLFrameSetElement>(*body))
> > +                return true;
> > +
> > +            auto* frameRenderer = ownerElement->renderer();
> > +            if (!frameRenderer)
> > +                return false;
> > +
> > +            // iframes should fill with a base color if the used color scheme of the
> > +            // element and the used color scheme of the embedded documentâs root
> > +            // element do not match.
> > +            if (frameView.useDarkAppearance() != frameRenderer->useDarkAppearance())
> > +                return !frameView.isTransparent();
> > +
> > +            return false;
> > +        }();
> > +
> > +        frameView.setContentIsOpaque(isOpaqueRoot);
> 
> Let's pull this root background paint into new function.

Moved the majority of the logic into a new function in RenderView.
Comment 12 Aditya Keerthi 2021-09-03 13:00:38 PDT
Comment on attachment 436937 [details]
Patch

Thanks for the review!
Comment 13 EWS 2021-09-03 13:33:29 PDT
Committed r282020 (241325@main): <https://commits.webkit.org/241325@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436937 [details].