[webkit-changes] [WebKit/WebKit] 060793: Switching light/dark modes doesn't update the base...
Simon Fraser
noreply at github.com
Tue Nov 19 19:21:15 PST 2024
Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 060793dcd5868ea0c16ad35818ba4ed8f2551d28
https://github.com/WebKit/WebKit/commit/060793dcd5868ea0c16ad35818ba4ed8f2551d28
Author: Simon Fraser <simon.fraser at apple.com>
Date: 2024-11-19 (Tue, 19 Nov 2024)
Changed paths:
M Source/WebCore/css/color/StyleColor.cpp
M Source/WebCore/css/color/StyleColor.h
M Source/WebCore/dom/Document.cpp
M Source/WebCore/dom/Document.h
M Source/WebCore/page/InteractionRegion.cpp
M Source/WebCore/page/LocalFrameView.cpp
M Source/WebCore/page/LocalFrameView.h
M Source/WebCore/rendering/RenderBox.cpp
M Source/WebCore/testing/Internals.cpp
Log Message:
-----------
Switching light/dark modes doesn't update the base background color if the root has `color` set explicitly
https://bugs.webkit.org/show_bug.cgi?id=283140
rdar://139917332
Reviewed by Aditya Keerthi.
The base background color is hardcoded to use a system color,
CSSValueAppleSystemControlBackground, in LocalFrameView. "Dark mode", "elevated user
interface" and color scheme state feed into the computation of the color returned for
CSSValueAppleSystemControlBackground. However, when those states change, there's no
renderer that sees a change to the background-color style, so no clear code path that
results in `LocalFrameView::updateBaseBackgroundColor()` being called.
`RenderBox::styleDidChange()` does call `frameView().recalculateBaseBackgroundColor()`
when it sees a style change for the document element or body renderers, but on light/dark
mode switches, that is triggered by chance because the `color` property (which is
inherited) changes, and we get a RepaintForText diff. If the content sets an explicit
color on the root, as the test case does, then the bug occurs (or at least it will once
fixing bug 267252 means that we don't get spurious render style diffs.)
Fix by calling `updateBaseBackgroundColorIfNecessary()` in `Document::resolveStyle().`
This needs to happen after style update, and before layout, because the color feeds
into scrollbar colors and compositing, both of which are consulted during/after layout.
I also tried to make this a "once per rendering update" thing, but that would require
extra dirty state tracking, and `updateBaseBackgroundColorIfNecessary()` is almost
always cheap, so it didn't seem necessary.
I also tried putting the base background color into the Document's RenderStyle, but
then it can inherit onto the document element, which is web-exposed.
* Source/WebCore/css/color/StyleColor.cpp:
(WebCore::operator<<): Make StyleColorOptions dumpable.
* Source/WebCore/css/color/StyleColor.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::resolveStyle):
(WebCore::Document::processColorScheme): No need to call `recalculateBaseBackgroundColor()` here, since
it will be called on the next style update.
* Source/WebCore/dom/Document.h: Drive-by: m_allowsColorSchemeTransformations was unused; remove it.
* Source/WebCore/page/InteractionRegion.cpp:
(WebCore::interactionRegionForRenderedRegion): Remove a log from an earlier commit.
* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::updateBaseBackgroundColorIfNecessary):
(WebCore::LocalFrameView::updateBackgroundRecursively):
(WebCore::LocalFrameView::recalculateBaseBackgroundColor): Deleted.
* Source/WebCore/page/LocalFrameView.h:
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::styleDidChange): No need to call `recalculateBaseBackgroundColor()` here.
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::setViewIsTransparent): call `setTransparent()`.
(WebCore::Internals::viewBaseBackgroundColor): Force a layout before getting the color.
Canonical link: https://commits.webkit.org/286833@main
To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications
More information about the webkit-changes
mailing list