Bug 225726

Summary:

[macOS] experimental "Use theme color for scroll area background" isn't working

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

New Bugs

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, hi, kondapallykalyan, pdr, simon.fraser, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Description Devin Rousso 2021-05-12 18:05:19 PDT

Comment 1 Devin Rousso 2021-05-12 18:05:35 PDT

<rdar://problem/77933000>

Comment 2 Devin Rousso 2021-05-12 18:09:07 PDT

Created attachment 428442 [details]
Patch

Comment 3 Tim Horton 2021-05-12 19:34:55 PDT

Comment on attachment 428442 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:-3997
> -                m_layerForOverhangAreas->setCustomAppearance(GraphicsLayer::CustomAppearance::ScrollingOverhang);

"CustomAppearance::ScrollingOverhang" can we get rid of this now?

Comment 4 Simon Fraser (smfr) 2021-05-12 20:38:05 PDT

Comment on attachment 428442 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3889
> +        if (page().settings().useThemeColorForScrollAreaBackgroundColor())
> +            backgroundColor = page().themeColor();
> +        if (page().settings().useSampledPageTopColorForScrollAreaBackgroundColor() && !backgroundColor.isValid())
> +            backgroundColor = page().sampledPageTopColor();
> +        if (!backgroundColor.isValid())
> +            backgroundColor = m_rootExtendedBackgroundColor;

Would be nicer to wrap this in a lambda that returns a Color.

>> Source/WebCore/rendering/RenderLayerCompositor.cpp:-3997
>> -                m_layerForOverhangAreas->setCustomAppearance(GraphicsLayer::CustomAppearance::ScrollingOverhang);
> 
> "CustomAppearance::ScrollingOverhang" can we get rid of this now?

It seems no? The backgroundShouldExtendBeyondPage setting has a default value of false.

Comment 5 Tim Horton 2021-05-12 21:09:49 PDT

Comment on attachment 428442 [details]
Patch

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

>>> Source/WebCore/rendering/RenderLayerCompositor.cpp:-3997
>>> -                m_layerForOverhangAreas->setCustomAppearance(GraphicsLayer::CustomAppearance::ScrollingOverhang);
>> 
>> "CustomAppearance::ScrollingOverhang" can we get rid of this now?
> 
> It seems no? The backgroundShouldExtendBeyondPage setting has a default value of false.

OH, I missed the code in updateLayerForOverhangAreasBackgroundColor that sets it, and thought this removed the setCustomAppearance() :D

Comment 6 Devin Rousso 2021-05-13 15:06:59 PDT

Created attachment 428567 [details]
Patch

Comment 7 EWS 2021-05-13 16:11:14 PDT

Committed r277459 (237703@main): <https://commits.webkit.org/237703@main>

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