Bug 228124

Summary: If the color scheme of an iframe differs from embedding document iframe gets an opaque canvas bg appropriate to its color scheme
Product: WebKit Reporter: Adam Argyle <argyle>
Component: CSSAssignee: Aditya Keerthi <akeerthi>
Status: RESOLVED FIXED    
Severity: Normal CC: akeerthi, changseok, clopez, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Provided demo url loaded in Safari, code is on top, and an empty demo are on bottom
none
`html { background-color: Canvas; }` fix
none
Patch
none
Patch none

Adam Argyle
Reported 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".
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
DetailsFormatted DiffDiff
Patch (12.39 KB, patch)
2021-08-31 13:48 PDT, Aditya Keerthi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 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?
Adam Argyle
Comment 2 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?
Simon Fraser (smfr)
Comment 3 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?
Simon Fraser (smfr)
Comment 5 2021-07-21 15:38:57 PDT
Nice!
Radar WebKit Bug Importer
Comment 6 2021-07-21 15:39:35 PDT
<rdar://problem/80922070>
Aditya Keerthi
Comment 7 2021-08-31 11:38:15 PDT
Created attachment 436918 [details] Patch
EWS Watchlist
Comment 8 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
Simon Fraser (smfr)
Comment 9 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.
Aditya Keerthi
Comment 10 2021-08-31 13:48:19 PDT
Created attachment 436937 [details] Patch
Aditya Keerthi
Comment 11 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.
Aditya Keerthi
Comment 12 2021-09-03 13:00:38 PDT
Comment on attachment 436937 [details] Patch Thanks for the review!
EWS
Comment 13 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].
Note You need to log in before you can comment on or make changes to this bug.