Bug 236580 – [CSS Container Queries] Implement full query parser and evaluator

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

So far we have been using the media query parser to parse container queries. It does not support nesting and other necessary features. Also the produced data structure are awkward to evaluate.

[Antti Koivisto]

Created attachment 451874 [details]
Patch

[Antti Koivisto]

Created attachment 451880 [details]
Patch

[Antti Koivisto]

Created attachment 451884 [details]
Patch

[Antti Koivisto]

Created attachment 451894 [details]
Patch

[Sam Weinig]

Comment on attachment 451894 [details]
Patch

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

> Source/WebCore/css/ContainerQuery.h:45
> +enum class ComparisonOperator : uint8_t { Less, LessOrEqual, Equal, Greater, GreaterOrEqual, True };

I would replace `Less` and `Greater` with `LessThan` and `GreaterThan` (also in the OrEqual variants) as that is usually what we call these.

> Source/WebCore/css/ContainerQueryParser.cpp:42
> +        bool isSizeQuery = equalIgnoringASCIICase(range.peek().value(), "size");

Could you instead do:

bool isSizeQuery = range.peek().functionId() == CSSValueSize;

?

> Source/WebCore/css/ContainerQueryParser.cpp:89
> +        if (equalIgnoringASCIICase(range.peek().value(), "not")) {

Again, can this be compared against the CSSValueNot ident rather than the explicit string check?

> Source/WebCore/css/ContainerQueryParser.cpp:143
> +    auto name = nameToken.value().toAtomString();

I think delaying the toAtomString() until it is needed might be a little more efficient and avoid an unnecessary atomization in some cases.

> Source/WebCore/css/ContainerQueryParser.cpp:146
> +        return CQ::SizeFeature { CQ::ComparisonOperator::True, name, { } };

I would WTFMove(name) here.

> Source/WebCore/css/ContainerQueryParser.cpp:161
> +            name = StringView(name).substring(4).toAtomString();

I can't exactly remember how this works with AtomStrings, but I think you want to instead do something like:

name = name.string().substring(4).toAtomString();

to potentially get the shared substring optimization.

> Source/WebCore/css/ContainerQueryParser.cpp:170
> +    return CQ::SizeFeature { op, name, length };

I would WTFMove(name) here.

> Source/WebCore/style/ContainerQueryEvaluator.cpp:186
> +    // FIXME: Support all features.

An evergreen comment :).

[Antti Koivisto]

Created attachment 451921 [details]
Patch for landing

[EWS]

Committed r289742 (247226@main): <https://commits.webkit.org/247226@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/88916234>