[webkit-reviews] review granted: [Bug 190526] Add <meta name="supported-color-schemes"> to control what color schemes the page supports : [Attachment 352193] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Timothy Hatcher <timothy at apple.com>'s
request for review:
Bug 190526: Add <meta name="supported-color-schemes"> to control what color
schemes the page supports
https://bugs.webkit.org/show_bug.cgi?id=190526

Attachment 352193: Patch

https://bugs.webkit.org/attachment.cgi?id=352193&action=review




--- Comment #13 from Dean Jackson <dino at apple.com> ---
Comment on attachment 352193
  --> https://bugs.webkit.org/attachment.cgi?id=352193
Patch

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

> Source/WebCore/dom/Document.cpp:3575
> +	       i++;

Why i++ here and ++i above? :)

> Source/WebCore/dom/Document.cpp:3594
> +    processColorSchemes(colorSchemes, [&supportedColorSchemes](StringView
key) {
> +	   if (equalLettersIgnoringASCIICase(key, "light"))
> +	       supportedColorSchemes.add(ColorSchemes::Light);
> +	   else if (equalLettersIgnoringASCIICase(key, "dark"))
> +	       supportedColorSchemes.add(ColorSchemes::Dark);
> +    });

I like this way of doing this parsing!

> Source/WebCore/dom/Document.h:898
> +    enum class ColorSchemes : uint8_t {

Shouldn't this be singular ColorScheme?

> LayoutTests/css-dark-mode/supported-color-schemes.html:68
> +    document.getElementById("meta").content = "dark";

More test suggestions
"     dark"
"dark	 "
"light, dark" (light should fail)
"foo dark"