Attachment 452170 Details for Bug 236702 – Patch

[patch] Patch

bug-236702-20220216130459.patch (text/plain), 17.74 KB, created by Antti Koivisto on 2022-02-16 03:05:00 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2022-02-16 03:05:00 PST

Size: 17.74 KB

patch

obsolete

>Subversion Revision: 289879
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9685dc65e79c5ed71559c84d96fec7b99af181af..f698df237da0dcb95fcc071a6dd7495db6c9740c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2022-02-16  Antti Koivisto  <antti@apple.com>
>+
>+        [CSS Container Queries] Size queries on unsupported axis should evaluate to unknown
>+        https://bugs.webkit.org/show_bug.cgi?id=236702
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        "If ... the query container does not support container size queries on the relevant axes,
>+        then the result of evaluating the size feature is unknown."
>+
>+        https://drafts.csswg.org/css-contain-3/#size-container
>+
>+        * style/ContainerQueryEvaluator.cpp:
>+        (WebCore::Style::ContainerQueryEvaluator::evaluate const):
>+
>+        Some cleanups.
>+
>+        (WebCore::Style::ContainerQueryEvaluator::evaluateQuery const):
>+        (WebCore::Style::ContainerQueryEvaluator::evaluateCondition const):
>+        (WebCore::Style::ContainerQueryEvaluator::evaluateSizeFeature const):
>+
>+        Check whether the container supports the feature axis and return Unknown of not.
>+
>+        * style/ContainerQueryEvaluator.h:
>+
>+        Rename EvaluationContext -> ResolvedContainer to be less generic.
>+
> 2022-02-15  Nikolaos Mouchtaris  <nmouchtaris@apple.com>
> 
>         Implement parsing and animation support for offset shorthand
>diff --git a/Source/WebCore/style/ContainerQueryEvaluator.cpp b/Source/WebCore/style/ContainerQueryEvaluator.cpp
>index 25a278ff0ffc405c0b9b44de16e2045d7ea830c8..dcf1ff4d262e98ad35a8960cd96d4663c44e980d 100644
>--- a/Source/WebCore/style/ContainerQueryEvaluator.cpp
>+++ b/Source/WebCore/style/ContainerQueryEvaluator.cpp
>@@ -37,8 +37,8 @@
> 
> namespace WebCore::Style {
> 
>-struct ContainerQueryEvaluator::EvaluationContext {
>-    RenderBox& renderer;
>+struct ContainerQueryEvaluator::ResolvedContainer {
>+    const RenderBox& renderer;
>     CSSToLengthConversionData conversionData;
> };
> 
>@@ -52,64 +52,64 @@ bool ContainerQueryEvaluator::evaluate(const FilteredContainerQuery& filteredCon
>     if (m_containers.isEmpty())
>         return false;
> 
>-    auto containerRendererForFilter = [&]() -> RenderBox* {
>+    auto makeResolvedContainer = [&](const RenderBox& renderer) -> ResolvedContainer {
>+        auto& view = renderer.view();
>+        return {
>+            renderer,
>+            CSSToLengthConversionData { &renderer.style(), &view.style(), nullptr, &view, 1 }
>+        };
>+    };
>+
>+    auto resolveContainer = [&]() -> std::optional<ResolvedContainer> {
>         for (auto& container : makeReversedRange(m_containers)) {
>             auto* renderer = dynamicDowncast<RenderBox>(container->renderer());
>             if (!renderer)
>-                return nullptr;
>+                return { };
>             if (filteredContainerQuery.nameFilter.isEmpty())
>-                return renderer;
>+                return makeResolvedContainer(*renderer);
>             // FIXME: Support type filter.
>             if (renderer->style().containerNames().contains(filteredContainerQuery.nameFilter))
>-                return renderer;
>+                return makeResolvedContainer(*renderer);
>         }
>-        return nullptr;
>+        return { };
>     };
> 
>-    auto* renderer = containerRendererForFilter();
>-    if (!renderer)
>-        return false;
>-
>-    auto& view = renderer->view();
>-    CSSToLengthConversionData { &renderer->style(), &view.style(), nullptr, &view, 1 };
>-
>-    EvaluationContext evaluationContext {
>-        *renderer,
>-        CSSToLengthConversionData { &renderer->style(), &view.style(), nullptr, &view, 1 }
>-    };
>+    auto container = resolveContainer();
>+    if (!container)
>+        return { };
> 
>-    return evaluateQuery(filteredContainerQuery.query, evaluationContext) == EvaluationResult::True;
>+    return evaluateQuery(filteredContainerQuery.query, *container) == EvaluationResult::True;
> }
> 
>-auto ContainerQueryEvaluator::evaluateQuery(const CQ::ContainerQuery& containerQuery, const EvaluationContext& context) const -> EvaluationResult
>+auto ContainerQueryEvaluator::evaluateQuery(const CQ::ContainerQuery& containerQuery, const ResolvedContainer& container) const -> EvaluationResult
> {
>     return WTF::switchOn(containerQuery, [&](const CQ::ContainerCondition& containerCondition) {
>-        return evaluateCondition(containerCondition, context);
>+        return evaluateCondition(containerCondition, container);
>     }, [&](const CQ::SizeQuery& sizeQuery) {
>-        return evaluateQuery(sizeQuery, context);
>+        return evaluateQuery(sizeQuery, container);
>     }, [&](const CQ::UnknownQuery&) {
>         return EvaluationResult::Unknown;
>     });
> }
> 
>-auto ContainerQueryEvaluator::evaluateQuery(const CQ::SizeQuery& sizeQuery, const EvaluationContext& context) const -> EvaluationResult
>+auto ContainerQueryEvaluator::evaluateQuery(const CQ::SizeQuery& sizeQuery, const ResolvedContainer& container) const -> EvaluationResult
> {
>     return WTF::switchOn(sizeQuery, [&](const CQ::SizeCondition& sizeCondition) {
>-        return evaluateCondition(sizeCondition, context);
>+        return evaluateCondition(sizeCondition, container);
>     }, [&](const CQ::SizeFeature& sizeFeature) {
>-        return evaluateSizeFeature(sizeFeature, context);
>+        return evaluateSizeFeature(sizeFeature, container);
>     });
> }
> 
> template<typename ConditionType>
>-auto ContainerQueryEvaluator::evaluateCondition(const ConditionType& condition, const EvaluationContext& context) const -> EvaluationResult
>+auto ContainerQueryEvaluator::evaluateCondition(const ConditionType& condition, const ResolvedContainer& container) const -> EvaluationResult
> {
>     if (condition.queries.isEmpty())
>         return EvaluationResult::Unknown;
> 
>     switch (condition.logicalOperator) {
>     case CQ::LogicalOperator::Not: {
>-        switch (evaluateQuery(condition.queries.first(), context)) {
>+        switch (evaluateQuery(condition.queries.first(), container)) {
>         case EvaluationResult::True:
>             return EvaluationResult::False;
>         case EvaluationResult::False:
>@@ -121,7 +121,7 @@ auto ContainerQueryEvaluator::evaluateCondition(const ConditionType& condition,
>     case CQ::LogicalOperator::And: {
>         auto result = EvaluationResult::True;
>         for (auto query : condition.queries) {
>-            auto queryResult = evaluateQuery(query, context);
>+            auto queryResult = evaluateQuery(query, container);
>             if (queryResult == EvaluationResult::False)
>                 return EvaluationResult::False;
>             if (queryResult == EvaluationResult::Unknown)
>@@ -132,7 +132,7 @@ auto ContainerQueryEvaluator::evaluateCondition(const ConditionType& condition,
>     case CQ::LogicalOperator::Or: {
>         auto result = EvaluationResult::False;
>         for (auto query : condition.queries) {
>-            auto queryResult = evaluateQuery(query, context);
>+            auto queryResult = evaluateQuery(query, container);
>             if (queryResult == EvaluationResult::True)
>                 return EvaluationResult::True;
>             if (queryResult == EvaluationResult::Unknown)
>@@ -160,7 +160,7 @@ static std::optional<LayoutUnit> computeSize(const CSSValue* value, const CSSToL
>     return primitiveValue.computeLength<LayoutUnit>(conversionData);
> }
> 
>-auto ContainerQueryEvaluator::evaluateSizeFeature(const CQ::SizeFeature& sizeFeature, const EvaluationContext& context) const -> EvaluationResult
>+auto ContainerQueryEvaluator::evaluateSizeFeature(const CQ::SizeFeature& sizeFeature, const ResolvedContainer& container) const -> EvaluationResult
> {
>     auto toEvaluationResult = [](bool boolean) {
>         return boolean ? EvaluationResult::True : EvaluationResult::False;
>@@ -188,27 +188,64 @@ auto ContainerQueryEvaluator::evaluateSizeFeature(const CQ::SizeFeature& sizeFea
>         if (sizeFeature.comparisonOperator == CQ::ComparisonOperator::True)
>             return toEvaluationResult(!!size);
> 
>-        auto expressionSize = computeSize(sizeFeature.value.get(), context.conversionData);
>+        auto expressionSize = computeSize(sizeFeature.value.get(), container.conversionData);
>         if (!expressionSize)
>             return EvaluationResult::Unknown;
> 
>         return toEvaluationResult(compare(size, *expressionSize));
>     };
> 
>-    if (sizeFeature.name == CQ::FeatureNames::width())
>-        return evaluateSize(context.renderer.contentWidth());
>+    enum class Axis : uint8_t { Both, Block, Inline, Width, Height };
>+    auto containerSupportsRequiredAxis = [&](Axis axis) {
>+        if (axis == Axis::Width)
>+            axis = container.renderer.isHorizontalWritingMode() ? Axis::Inline : Axis::Block;
>+        else if (axis == Axis::Height)
>+            axis = container.renderer.isHorizontalWritingMode() ? Axis::Block : Axis::Inline;
>+
>+        switch (container.renderer.style().containerType()) {
>+        case ContainerType::Size:
>+            return true;
>+        case ContainerType::InlineSize:
>+            return axis == Axis::Inline;
>+        case ContainerType::None:
>+            ASSERT_NOT_REACHED();
>+            return false;
>+        }
>+    };
> 
>-    if (sizeFeature.name == CQ::FeatureNames::height())
>-        return evaluateSize(context.renderer.contentHeight());
>+    if (sizeFeature.name == CQ::FeatureNames::width()) {
>+        if (!containerSupportsRequiredAxis(Axis::Width))
>+            return EvaluationResult::Unknown;
> 
>-    if (sizeFeature.name == CQ::FeatureNames::inlineSize())
>-        return evaluateSize(context.renderer.contentLogicalWidth());
>+        return evaluateSize(container.renderer.contentWidth());
>+    }
> 
>-    if (sizeFeature.name == CQ::FeatureNames::blockSize())
>-        return evaluateSize(context.renderer.contentLogicalHeight());
>+    if (sizeFeature.name == CQ::FeatureNames::height()) {
>+        if (!containerSupportsRequiredAxis(Axis::Height))
>+            return EvaluationResult::Unknown;
>+
>+        return evaluateSize(container.renderer.contentHeight());
>+    }
>+
>+    if (sizeFeature.name == CQ::FeatureNames::inlineSize()) {
>+        if (!containerSupportsRequiredAxis(Axis::Inline))
>+            return EvaluationResult::Unknown;
>+
>+        return evaluateSize(container.renderer.contentLogicalWidth());
>+    }
>+
>+    if (sizeFeature.name == CQ::FeatureNames::blockSize()) {
>+        if (!containerSupportsRequiredAxis(Axis::Block))
>+            return EvaluationResult::Unknown;
>+
>+        return evaluateSize(container.renderer.contentLogicalHeight());
>+    }
> 
>     if (sizeFeature.name == CQ::FeatureNames::aspectRatio()) {
>-        auto boxRatio = context.renderer.contentWidth().toDouble() / context.renderer.contentHeight().toDouble();
>+        if (!containerSupportsRequiredAxis(Axis::Both))
>+            return EvaluationResult::Unknown;
>+
>+        auto boxRatio = container.renderer.contentWidth().toDouble() / container.renderer.contentHeight().toDouble();
>         if (sizeFeature.comparisonOperator == CQ::ComparisonOperator::True)
>             return toEvaluationResult(!!boxRatio);
> 
>@@ -231,12 +268,15 @@ auto ContainerQueryEvaluator::evaluateSizeFeature(const CQ::SizeFeature& sizeFea
>     }
> 
>     if (sizeFeature.name == CQ::FeatureNames::orientation()) {
>+        if (!containerSupportsRequiredAxis(Axis::Both))
>+            return EvaluationResult::Unknown;
>+
>         if (!is<CSSPrimitiveValue>(sizeFeature.value) || sizeFeature.comparisonOperator != CQ::ComparisonOperator::Equal)
>             return EvaluationResult::Unknown;
> 
>         auto& value = downcast<CSSPrimitiveValue>(*sizeFeature.value);
> 
>-        bool isPortrait = context.renderer.contentHeight() >= context.renderer.contentWidth();
>+        bool isPortrait = container.renderer.contentHeight() >= container.renderer.contentWidth();
>         if (value.valueID() == CSSValuePortrait)
>             return toEvaluationResult(isPortrait);
>         if (value.valueID() == CSSValueLandscape)
>diff --git a/Source/WebCore/style/ContainerQueryEvaluator.h b/Source/WebCore/style/ContainerQueryEvaluator.h
>index 99be222d15d9f9d1ab07a386741d23b29f499cd6..cfe46ad03c3a65585c5eeaff5a6f75205b397e7a 100644
>--- a/Source/WebCore/style/ContainerQueryEvaluator.h
>+++ b/Source/WebCore/style/ContainerQueryEvaluator.h
>@@ -40,13 +40,13 @@ public:
>     bool evaluate(const FilteredContainerQuery&) const;
> 
> private:
>-    struct EvaluationContext;
>+    struct ResolvedContainer;
>     enum class EvaluationResult : uint8_t { False, True, Unknown };
> 
>-    EvaluationResult evaluateQuery(const CQ::ContainerQuery&, const EvaluationContext&) const;
>-    EvaluationResult evaluateQuery(const CQ::SizeQuery&, const EvaluationContext&) const;
>-    template<typename ConditionType> EvaluationResult evaluateCondition(const ConditionType&, const EvaluationContext&) const;
>-    EvaluationResult evaluateSizeFeature(const CQ::SizeFeature&, const EvaluationContext&) const;
>+    EvaluationResult evaluateQuery(const CQ::ContainerQuery&, const ResolvedContainer&) const;
>+    EvaluationResult evaluateQuery(const CQ::SizeQuery&, const ResolvedContainer&) const;
>+    template<typename ConditionType> EvaluationResult evaluateCondition(const ConditionType&, const ResolvedContainer&) const;
>+    EvaluationResult evaluateSizeFeature(const CQ::SizeFeature&, const ResolvedContainer&) const;
> 
>     const Vector<Ref<const Element>>& m_containers;
> };
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index ac6cd6e16eebcaeff138df053c3801831e125e16..2134f155c33a31352804dcd5e95bbdcca95fb115 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,14 @@
>+2022-02-16  Antti Koivisto  <antti@apple.com>
>+
>+        [CSS Container Queries] Size queries on unsupported axis should evaluate to unknown
>+        https://bugs.webkit.org/show_bug.cgi?id=236702
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/css/css-contain/container-queries/aspect-ratio-feature-evaluation-expected.txt:
>+        * web-platform-tests/css/css-contain/container-queries/container-selection-expected.txt:
>+        * web-platform-tests/css/css-contain/container-queries/unsupported-axis-expected.txt:
>+
> 2022-02-15  Nikolaos Mouchtaris  <nmouchtaris@apple.com>
> 
>         Implement parsing and animation support for offset shorthand
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/aspect-ratio-feature-evaluation-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/aspect-ratio-feature-evaluation-expected.txt
>index 1756eefaff4957d2ec93a1ca0e1bfb0c6947c7a6..08aee9ab567dc425370de0e55ca38b04532d986d 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/aspect-ratio-feature-evaluation-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/aspect-ratio-feature-evaluation-expected.txt
>@@ -1,4 +1,4 @@
> 
>-FAIL @container queries with aspect-ratio and size containment assert_equals: Should not match for inline-size containment expected "rgb(255, 0, 0)" but got "rgb(0, 128, 0)"
>+PASS @container queries with aspect-ratio and size containment
> FAIL @container query with aspect-ratio change after resize assert_equals: Should match 2/1 min-ratio expected "rgb(0, 255, 0)" but got "rgba(0, 0, 0, 0)"
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-selection-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-selection-expected.txt
>index 2d6db34b9445de1bbafca6c3db0e20a970c23348..5747fe4aa448923a61f31b99e609c829945f116b 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-selection-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/container-selection-expected.txt
>@@ -2,7 +2,7 @@
> PASS size(width: 16px) for .size > .inline > span
> PASS size(height: 16px) for .inline > .size > span
> PASS size(width: 16px) for .inline > .size > span
>-FAIL size(height) for .size > .inline > span assert_equals: expected "" but got "true"
>+PASS size(height) for .size > .inline > span
> FAIL type(inline-size) size(width: 16px) for .inline > .size > span assert_equals: expected "true" but got ""
> FAIL type(inline-size) size(width: 16px) for .size > .inline > span assert_equals: expected "true" but got ""
> FAIL type(size) size(height: 16px) for .inline > .size > span assert_equals: expected "true" but got ""
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/unsupported-axis-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/unsupported-axis-expected.txt
>index 2f6d071dc08bef843bd0fcfc882f90565b7f5c9e..d0ac7e25d7c04e042f50c5a64fac4231bbf52f0a 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/unsupported-axis-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/unsupported-axis-expected.txt
>@@ -1,14 +1,14 @@
> Test
> 
> PASS size(width > 0px)
>-FAIL size(height > 0px) assert_equals: expected "" but got "true"
>+PASS size(height > 0px)
> PASS size((height > 0px) or (width > 0px))
> PASS size((width > 0px) or (height > 0px))
> PASS size((orientation: landscape) or (width > 0px))
> PASS size((width > 0px) or (orientation: landscape))
>-FAIL size((height > 0px) or (orientation: landscape)) assert_equals: expected "" but got "true"
>-FAIL size((height > 0px) or (orientation: landscape)), with contain:size assert_equals: expected "" but got "true"
>+PASS size((height > 0px) or (orientation: landscape))
>+PASS size((height > 0px) or (orientation: landscape)), with contain:size
> PASS size(inline-size > 0px)
>-FAIL size(block-size > 0px) assert_equals: expected "" but got "true"
>-FAIL size(block-size > 0px), with writing-mode:vertical-rl assert_equals: expected "" but got "true"
>+PASS size(block-size > 0px)
>+PASS size(block-size > 0px), with writing-mode:vertical-rl
>

Actions: View | Formatted Diff | Diff

Attachments on bug 236702: 452170 | 452186