[webkit-changes] [WebKit/WebKit] d4ec30: REGRESSION (259663 at main): lowes.com: Product image...

[Sammy Gill]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d4ec309d5e6769e10098079b5b9c09ae917418a0
      https://github.com/WebKit/WebKit/commit/d4ec309d5e6769e10098079b5b9c09ae917418a0
  Author: Sammy Gill <sammy.gill at apple.com>
  Date:   2023-03-30 (Thu, 30 Mar 2023)

  Changed paths:
    A LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/grid-item-image-percentage-min-height-computes-as-0-expected.html
    A LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/grid-item-image-percentage-min-height-computes-as-0.html
    A LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/nested-flexbox-image-percentage-max-height-computes-as-none-expected.html
    A LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/nested-flexbox-image-percentage-max-height-computes-as-none.html
    M Source/WebCore/rendering/RenderBox.cpp

  Log Message:
  -----------
  REGRESSION (259663 at main): lowes.com: Product image is blank
https://bugs.webkit.org/show_bug.cgi?id=254603
rdar://106926883

Reviewed by Alan Baradlay.

The product image on Lowes had a structure that is very similar
to the following that results in the product image not rendering
correctly (having a width and height of 0). I made some slight
modifications to make it easier to digest, but the following is
indicative of the issue that is causing the image to show up as blank.

body {
  height: 50px;
}
img {
  position: relative;
  max-width: 100%;
  max-height: 100%;
}
.outer-flexbox {
  display: flex;
  width: 100%;
  height: 100%;
}
.outer-flexbox-item {
  position: relative;
  min-width: 100%;
}
.inner-flexbox {
  position: absolute;
  display: flex;
  inset: 0px;
}
</style>

<div class="outer-flexbox">
    <div class="outer-flexbox-item">
        <div class="inner-flexbox">
            <div>
                <img src="/css/support/60x60-green.png">
            </div>
        </div>
    </div>
</div>

The problem arises when we layout the outer-flexbox and eventually
recurse into the image to compute its preferred width. During this
process, we attempt to compute the max-height by resolving the percentage value, but we end up
incorrectly computing a max-height of 0. This max-height computation is
done when we reach RenderBox::computeLogicalHeightUsing and end up
calling RenderBox::computeReplacedLogicalHeightUsing with a heightType
of MaxSize. computeReplacedLogicalHeightUsing will calculate this height
differently depending on the LengthType of the passed in height and in the
case of this scenario we fall into the LengthType::Percent case for
max-height. Since this code is unable to resolve this height (due to the
fact its containing block depends on the size of its content), it
returns a value of 0. This 0 value ends up affecting not only the size
of the image in both the width and height dimensions, but also affects
the flex item of the inner flexbox and the size of the inner flexbox as
part of a flex item for the outer flexbox.

The solution here is to modify computeLogicalHeightUsing to check if
the min/max height would compute to 0/none depending on the heightType.
Ideally, the caller should not have to do this (like how it is done in
RenderBox::computeReplacedLogicalHeightRespectingMinMaxHeight), and
this would be done handled in computeReplacedLogicalHeightUsing, but
a couple of call sites make this type of change tricky. It is
particularly when computeReplacedLogicalHeightUsing with a heightType of
MainOrPreferredSize since these call sites expect the function to return
a definite value, so it is not clear what would be the correct logic if
it instead returned an empty optional.

To accomplish this, we can use the existing helper function
replacedMinMaxLogicalHeightComputesAsNone, which returns true in this
example to indicate that the used value of max-height should be treated
as none. This was actually already being used in the other call site:
computeReplacedLogicalHeightRespectingMinMaxHeight. To make it more
difficult to call this function when replacedMinMaxLogicalHeightComputesAsNone
returns false, I added an assert to trigger in that problematic scenario.

* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/grid-item-image-percentage-min-height-computes-as-0-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/grid-item-image-percentage-min-height-computes-as-0.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/nested-flexbox-image-percentage-max-height-computes-as-none-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/nested-flexbox-image-percentage-max-height-computes-as-none.html: Added.
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::computeLogicalHeightUsing const):
(WebCore::RenderBox::computeReplacedLogicalHeightUsing const):

Canonical link: https://commits.webkit.org/262342@main