Bug 226438 – Add additional target luminance keywords for color-contrast() added for https://github.com/w3c/csswg-drafts/issues/6311

• Component: CSS
• Status: RESOLVED FIXED

[Sam Weinig]

AAA (7.0) and AAA-large (4.5) were added for https://github.com/w3c/csswg-drafts/issues/6311 / https://github.com/w3c/csswg-drafts/commit/45b0b891e81ff308b4ce482ac1e3621382608f0a#diff-2035345378942a021190ce428e1f91413c366fec038ea7c90ec1a446100c4f5e.

[Sam Weinig]

Created attachment 430153 [details]
Patch

[Darin Adler]

Comment on attachment 430153 [details]
Patch

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

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1618
> +                auto id = args.consumeIncludingWhitespace().id();
> +                for (auto& [identifier, luminance] : targetLuminanceMappings) {
> +                    if (identifier == id)
> +                        return luminance;
> +                }
> +                return std::nullopt;

Since these keys are sorted, you could have used SortedArrayMap and written this:

    constexpr SortedArrayMap targetLuminanceMap { targetLuminanceMappings };
    return targetLuminanceMap.tryGet(args.consumeIncludingWhitespace().id());

However, it returns a const double*, not a std::optional<double>. But in other ways it’s designed just for uses like this. Switches to binary search if the list gets long.

[EWS]

Committed r278262 (238299@main): <https://commits.webkit.org/238299@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/78670255>

[Sam Weinig]

(In reply to Darin Adler from comment #2)
> Comment on attachment 430153 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430153&action=review
> 
> > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1618
> > +                auto id = args.consumeIncludingWhitespace().id();
> > +                for (auto& [identifier, luminance] : targetLuminanceMappings) {
> > +                    if (identifier == id)
> > +                        return luminance;
> > +                }
> > +                return std::nullopt;
> 
> Since these keys are sorted, you could have used SortedArrayMap and written
> this:
> 
>     constexpr SortedArrayMap targetLuminanceMap { targetLuminanceMappings };
>     return targetLuminanceMap.tryGet(args.consumeIncludingWhitespace().id());
> 
> However, it returns a const double*, not a std::optional<double>. But in
> other ways it’s designed just for uses like this. Switches to binary search
> if the list gets long.

Missed this. Sorry. Addressed in https://bugs.webkit.org/show_bug.cgi?id=226444.