Skip to content

Conversation

pxlcoder
Copy link
Member

@pxlcoder pxlcoder commented Sep 6, 2023

5413aa7

Default link colors lack sufficient contrast in the dark color scheme
https://bugs.webkit.org/show_bug.cgi?id=209851
rdar://61149466

Reviewed by Tim Nguyen and Megan Gardner.

Currently, link colors are the same in both the light and dark color schemes.
This results in a lack of contrast against dark backgrounds.

To fix, dark-mode specific link colors are defined. The definitions match the
values used by Chromium, and are not taken from the system link color. This
approach was chosen as the default link color in the light color scheme is
defined by https://html.spec.whatwg.org/multipage/rendering.html#phrasing-content-3,
and also does not come from the system.

* LayoutTests/css-dark-mode/link-colors-expected.html: Added.
* LayoutTests/css-dark-mode/link-colors.html: Added.
* Source/WebCore/dom/Document.cpp:

Link colors can be adjusted by HTML using the `link`, `alink`, and `vlink`
attributes. Adjust logic to ensure that a static link color is only used if the
corresponding attribute is specified. Without these changes, the dark mode
color would never be used, as the light mode color is stored as the default.

(WebCore::Document::linkColor const):
(WebCore::Document::visitedLinkColor const):
(WebCore::Document::activeLinkColor const):
(WebCore::Document::resetLinkColor):
(WebCore::Document::resetVisitedLinkColor):
(WebCore::Document::resetActiveLinkColor):
* Source/WebCore/dom/Document.h:
(WebCore::Document::linkColor const): Deleted.
(WebCore::Document::visitedLinkColor const): Deleted.
(WebCore::Document::activeLinkColor const): Deleted.
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::RenderTheme::systemColor const):

Add definitions for colors under the dark color scheme.

* Source/WebCore/style/ColorFromPrimitiveValue.cpp:
(WebCore::Style::colorFromPrimitiveValue):

Pass in `StyleColorOptions` when determining the link color, so that it can be
resolved against the used color scheme.

Canonical link: https://commits.webkit.org/267847@main

bfd7c04

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ⏳ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ⏳ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ⏳ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@pxlcoder pxlcoder self-assigned this Sep 6, 2023
@pxlcoder pxlcoder added the CSS Cascading Style Sheets implementation label Sep 6, 2023
@pxlcoder pxlcoder marked this pull request as ready for review September 8, 2023 22:00
Comment on lines 15 to 26
<a class="light" href="https://www.webkit.org">Light</a>
<a class="dark" href="https://www.webkit.org">Dark</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also add tests for :visited by doing <a href="">

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated – thanks!

@hortont424
Copy link
Contributor

constrast!

@pxlcoder pxlcoder added the merge-queue Applied to send a pull request to merge-queue label Sep 11, 2023
https://bugs.webkit.org/show_bug.cgi?id=209851
rdar://61149466

Reviewed by Tim Nguyen and Megan Gardner.

Currently, link colors are the same in both the light and dark color schemes.
This results in a lack of contrast against dark backgrounds.

To fix, dark-mode specific link colors are defined. The definitions match the
values used by Chromium, and are not taken from the system link color. This
approach was chosen as the default link color in the light color scheme is
defined by https://html.spec.whatwg.org/multipage/rendering.html#phrasing-content-3,
and also does not come from the system.

* LayoutTests/css-dark-mode/link-colors-expected.html: Added.
* LayoutTests/css-dark-mode/link-colors.html: Added.
* Source/WebCore/dom/Document.cpp:

Link colors can be adjusted by HTML using the `link`, `alink`, and `vlink`
attributes. Adjust logic to ensure that a static link color is only used if the
corresponding attribute is specified. Without these changes, the dark mode
color would never be used, as the light mode color is stored as the default.

(WebCore::Document::linkColor const):
(WebCore::Document::visitedLinkColor const):
(WebCore::Document::activeLinkColor const):
(WebCore::Document::resetLinkColor):
(WebCore::Document::resetVisitedLinkColor):
(WebCore::Document::resetActiveLinkColor):
* Source/WebCore/dom/Document.h:
(WebCore::Document::linkColor const): Deleted.
(WebCore::Document::visitedLinkColor const): Deleted.
(WebCore::Document::activeLinkColor const): Deleted.
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::RenderTheme::systemColor const):

Add definitions for colors under the dark color scheme.

* Source/WebCore/style/ColorFromPrimitiveValue.cpp:
(WebCore::Style::colorFromPrimitiveValue):

Pass in `StyleColorOptions` when determining the link color, so that it can be
resolved against the used color scheme.

Canonical link: https://commits.webkit.org/267847@main
@webkit-commit-queue
Copy link
Collaborator

Committed 267847@main (5413aa7): https://commits.webkit.org/267847@main

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants