| Summary: | Add additional target luminance keywords for color-contrast() added for https://github.com/w3c/csswg-drafts/issues/6311 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||
| Component: | CSS | Assignee: | Sam Weinig <sam> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | cdumez, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, macpherson, menard, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | Other | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Sam Weinig
2021-05-30 15:29:23 PDT
Created attachment 430153 [details]
Patch
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. 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]. (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. |