Skip to content

Commit

Permalink
[css-contain-intrinsic-size] Apply the value of css contain-intrinsic…
Browse files Browse the repository at this point in the history
…-size to <select>

https://bugs.webkit.org/show_bug.cgi?id=246338

Reviewed by Alan Baradlay.

Per [1], the intrinsic size is determined as if the element had no content.
When RenderMenuList without content, it should still display the appearance
of theme. So if it is a size containment, it should have size of theme appearance.
While calculating height, we should keep the height from theme style.

Per [2], contain-intrinsic-* properties specify an explicit intrinsic inner size.
For RenderMenuList, the explicit intrinsic inner size includes the content size and
the theme size. So while calculating height, we should override the intrinsic size.

When adjusting the style for themes, it might set a fixed value to properties, like width,
height, min-width and min-height. This would make contain-intrinsic-* properties not
effective. To fix this, the values of these properties need to restore to the auto value.

[1] https://www.w3.org/TR/css-contain-2/#containment-size
[2] https://www.w3.org/TR/css-sizing-4/#intrinsic-size-override

* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-032-expected.txt:
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::overrideLogicalHeightForSizeContainment): Override the height and handle RenderMenuList.
(WebCore::RenderBox::updateLogicalHeight):
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjust const):
(WebCore::Style::Adjuster::adjustThemeStyle const): If there is contain-intrinsic-size and the css size properties are
changed from auto to a fixed value, we should restore them, so that contain-intrinsic-size would be effective.
* Source/WebCore/style/StyleAdjuster.h:

Canonical link: https://commits.webkit.org/257704@main
  • Loading branch information
cathiechen committed Dec 11, 2022
1 parent d3b4d2e commit b727666
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 29 deletions.
@@ -1,24 +1,12 @@


FAIL .test 1 assert_equals:
<select class="test ciw-none cih-none" data-expected-client-width="34" data-expected-client-height="16"></select>
clientHeight expected 16 but got 12
FAIL .test 2 assert_equals:
<select class="test ciw-none cih-0" data-expected-client-width="34" data-expected-client-height="0"></select>
clientHeight expected 0 but got 12
PASS .test 1
PASS .test 2
PASS .test 3
FAIL .test 4 assert_equals:
<select class="test ciw-0 cih-none" data-expected-client-width="0" data-expected-client-height="16"></select>
clientHeight expected 16 but got 12
FAIL .test 5 assert_equals:
<select class="test ciw-0 cih-0" data-expected-client-width="0" data-expected-client-height="0"></select>
clientHeight expected 0 but got 12
PASS .test 4
PASS .test 5
PASS .test 6
FAIL .test 7 assert_equals:
<select class="test ciw-100 cih-none" data-expected-client-width="100" data-expected-client-height="16"></select>
clientHeight expected 16 but got 12
FAIL .test 8 assert_equals:
<select class="test ciw-100 cih-0" data-expected-client-width="100" data-expected-client-height="0"></select>
clientHeight expected 0 but got 12
PASS .test 7
PASS .test 8
PASS .test 9

31 changes: 21 additions & 10 deletions Source/WebCore/rendering/RenderBox.cpp
Expand Up @@ -2977,18 +2977,29 @@ void RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit height)
downcast<RenderFlexibleBox>(parent())->setCachedChildIntrinsicContentLogicalHeight(*this, height);
}

void RenderBox::overrideLogicalHeightForSizeContainment()
{
LayoutUnit intrinsicHeight;
if (auto height = explicitIntrinsicInnerLogicalHeight())
intrinsicHeight = height.value();
else if (isMenuList()) {
// RenderMenuList has its own theme, if there isn't explicitIntrinsicInnerLogicalHeight,
// as a size containment, it should be treated as if there is no content, and the height
// should the original logical height for theme.
return;
}

// We need the exact width of border and padding here, yet we can't use borderAndPadding* interfaces.
// Because these interfaces evetually call borderAfter/Before, and RenderBlock::borderBefore
// adds extra border to fieldset by adding intrinsicBorderForFieldset which is not needed here.
auto borderAndPadding = RenderBox::borderBefore() + RenderBox::paddingBefore() + RenderBox::borderAfter() + RenderBox::paddingAfter();
setLogicalHeight(intrinsicHeight + borderAndPadding + scrollbarLogicalHeight());
}

void RenderBox::updateLogicalHeight()
{
if (shouldApplySizeContainment() && !isRenderGrid()) {
// We need the exact width of border and padding here, yet we can't use borderAndPadding* interfaces.
// Because these interfaces evetually call borderAfter/Before, and RenderBlock::borderBefore
// adds extra border to fieldset by adding intrinsicBorderForFieldset which is not needed here.
LayoutUnit intrinsicHeight;
if (auto height = explicitIntrinsicInnerLogicalHeight())
intrinsicHeight = height.value();
auto borderAndPadding = RenderBox::borderBefore() + RenderBox::paddingBefore() + RenderBox::borderAfter() + RenderBox::paddingAfter();
setLogicalHeight(intrinsicHeight + borderAndPadding + scrollbarLogicalHeight());
}
if (shouldApplySizeContainment() && !isRenderGrid())
overrideLogicalHeightForSizeContainment();

cacheIntrinsicContentLogicalHeightForFlexItem(contentLogicalHeight());
auto computedValues = computeLogicalHeight(logicalHeight(), logicalTop());
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/rendering/RenderBox.h
Expand Up @@ -432,6 +432,8 @@ override;
void updateLogicalHeight();
virtual LogicalExtentComputedValues computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logicalTop) const;

void overrideLogicalHeightForSizeContainment();

void cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit) const;

// This function will compute the logical border-box height, without laying
Expand Down
28 changes: 27 additions & 1 deletion Source/WebCore/style/StyleAdjuster.cpp
Expand Up @@ -519,7 +519,7 @@ void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearance

// Let the theme also have a crack at adjusting the style.
if (style.hasAppearance())
RenderTheme::singleton().adjustStyle(style, m_element, userAgentAppearanceStyle);
adjustThemeStyle(style, userAgentAppearanceStyle);

// If we have first-letter pseudo style, do not share this style.
if (style.hasPseudoStyle(PseudoId::FirstLetter))
Expand Down Expand Up @@ -725,6 +725,32 @@ void Adjuster::adjustAnimatedStyle(RenderStyle& style, OptionSet<AnimationImpact
style.setUsedZIndex(0);
}

void Adjuster::adjustThemeStyle(RenderStyle& style, const RenderStyle* userAgentAppearanceStyle) const
{
ASSERT(style.hasAppearance());
auto isOldWidthAuto = style.width().isAuto();
auto isOldMinWidthAuto = style.minWidth().isAuto();
auto isOldHeightAuto = style.height().isAuto();
auto isOldMinHeightAuto = style.minHeight().isAuto();

RenderTheme::singleton().adjustStyle(style, m_element, userAgentAppearanceStyle);

if (style.containsSize()) {
if (style.containIntrinsicWidthType() != ContainIntrinsicSizeType::None) {
if (isOldWidthAuto)
style.setWidth(Length(LengthType::Auto));
if (isOldMinWidthAuto)
style.setMinWidth(Length(LengthType::Auto));
}
if (style.containIntrinsicHeightType() != ContainIntrinsicSizeType::None) {
if (isOldHeightAuto)
style.setHeight(Length(LengthType::Auto));
if (isOldMinHeightAuto)
style.setMinHeight(Length(LengthType::Auto));
}
}
}

void Adjuster::adjustForSiteSpecificQuirks(RenderStyle& style) const
{
if (!m_element)
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/style/StyleAdjuster.h
Expand Up @@ -71,6 +71,9 @@ class Adjuster {
private:
void adjustDisplayContentsStyle(RenderStyle&) const;
void adjustForSiteSpecificQuirks(RenderStyle&) const;

void adjustThemeStyle(RenderStyle&, const RenderStyle* userAgentAppearanceStyle) const;

static OptionSet<EventListenerRegionType> computeEventListenerRegionTypes(const Document&, const RenderStyle&, const EventTarget&, OptionSet<EventListenerRegionType>);

const Document& m_document;
Expand Down

0 comments on commit b727666

Please sign in to comment.