Skip to content
Permalink
Browse files
Flex boxes (both old and new) don't handle max-height images correctly.
https://bugs.webkit.org/show_bug.cgi?id=118000

Reviewed by Beth Dakin.

Source/WebCore:

Tests: css3/flexbox/image-percent-max-height.html
       fast/flexbox/image-percent-max-height.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::dirtyForLayoutFromPercentageHeightDescendants):
(WebCore::RenderBlock::layoutBlockChildren):
Pull the percentage height descendant code that dirties those descendants
out of layoutBlockChildren and into a protected helper function,
dirtyForLayoutFromPercentageHeightDescendants, that can be called from the
flex box code.

Also patch dirtyForLayoutFromPercentageHeightDescendants so that it will dirty
preferred logical widths when a child has an aspect ratio, since we know that
percentage height changes will potentially affect the preferred widths of the image and
its ancestor blocks.

* rendering/RenderBlock.h:
Declaration of the new dirtyForLayoutFromPercentageHeightDescendants function.

* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
Make the old flex box code call dirtyForLayoutFromPercentageHeightDescendants so
that everything is dirtied properly.

(WebCore::RenderDeprecatedFlexibleBox::layoutHorizontalBox):
(WebCore::RenderDeprecatedFlexibleBox::layoutVerticalBox):
Remove the isReplaced()/percentage height/width dirtying now that the old flexible
box is using the same dirtying mechanism as RenderBlock.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
Patch the new flexible box code to use the dirtying mechanism that RenderBlock
uses for percentage heights/widths on replaced descendants.

* rendering/RenderObject.h:
(WebCore::RenderObject::hasAspectRatio):
Pulled the static helper function from RenderReplaced into a full-blown method
on RenderObject, so that dirtyForLayoutFromPercentageHeightDescendants can call
it to check if an object has an aspect ratio.

* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeAspectRatioInformationForRenderBox):
(WebCore::RenderReplaced::computeIntrinsicRatioInformation):
Patch the call sites of the static helper function to use hasAspectRatio instead
and get rid of the static in the cpp file.

LayoutTests:

* css3/flexbox/image-percent-max-height-expected.html: Added.
* css3/flexbox/image-percent-max-height.html: Added.
* css3/flexbox/resources/hero.png: Added.
* fast/flexbox/image-percent-max-height-expected.html: Added.
* fast/flexbox/image-percent-max-height.html: Added.
* fast/flexbox/resources/hero.png: Added.



Canonical link: https://commits.webkit.org/136132@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@151997 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
David Hyatt committed Jun 26, 2013
1 parent 8eaf29a commit 77e251c91460bc385e024d01d5db60a44f8d2f75
@@ -1,3 +1,17 @@
2013-06-25 David Hyatt <hyatt@apple.com>

Flex boxes (both old and new) don't handle max-height images correctly.
https://bugs.webkit.org/show_bug.cgi?id=118000

Reviewed by Beth Dakin.

* css3/flexbox/image-percent-max-height-expected.html: Added.
* css3/flexbox/image-percent-max-height.html: Added.
* css3/flexbox/resources/hero.png: Added.
* fast/flexbox/image-percent-max-height-expected.html: Added.
* fast/flexbox/image-percent-max-height.html: Added.
* fast/flexbox/resources/hero.png: Added.

2013-06-26 Ryosuke Niwa <rniwa@webkit.org>

editing/selection/doubleclick-crash.html can be flaky
@@ -0,0 +1,25 @@
<style>

.wrapper {
top: 0;
left: 0;
position: absolute;
width: 100%;
height:100%;

background: #080;
}

img {
height:400px;
background: #00F;
margin-left:auto;
margin-right:auto;
display:block;
}

</style>

<div id="test" style="width:400px;height:400px; background-color:#080">
<img id="between" src="resources/hero.png">
</div>
@@ -0,0 +1,39 @@
<style>

.wrapper {
top: 0;
left: 0;
position: absolute;
width: 100%;
height:100%;

display: -webkit-flex;
-webkit-flex-orientation: horizontal;
background: #080;
}

img {
max-width: 100%;
max-height:100%;
background: #00F;
margin-left:auto;
margin-right:auto;
margin-top:auto;
margin-bottom:auto;
}

</style>

<div id="test" style="width:500px;height:500px;position:relative">
<div class="wrapper">
<img id="between" src="resources/hero.png">
</div>
</div>

<script>
document.body.offsetLeft
document.getElementById('test').style.width = '400px'
document.getElementById('test').style.height = '400px'
document.body.offsetLeft
</script>

Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,27 @@
<style>

.wrapper {
top: 0;
left: 0;
position: absolute;
width: 100%;
height:100%;

display:-webkit-box;
-webkit-box-orient: horizontal;
-webkit-box-pack: center;
-webkit-box-align: center;

background: #080;
}

img {
height:400px;
background: #00F;
}

</style>

<div id="test" style="width:400px;height:400px; background-color:#080; position:relative">
<div class="wrapper"><img id="between" src="resources/hero.png"></div>
</div>
@@ -0,0 +1,36 @@
<!doctype html>
<style>

.wrapper {
top: 0;
left: 0;
position: absolute;
width: 100%;
height:100%;

display: -webkit-box;
-webkit-box-orient:horizontal;
-webkit-box-pack: center;
-webkit-box-align: stretch;
background: #080;
}

img {
max-width: 100%;
height:100%;
background: #00F;
}

</style>

<div id="test" style="width:500px;height:500px;position:relative">
<div class="wrapper"><img id="between" src="resources/hero.png" style="vertical-align:top"></div>
</div>

<script>
document.body.offsetLeft
document.getElementById('test').style.width = '400px'
document.getElementById('test').style.height = '400px'
document.body.offsetLeft
</script>

Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -1,4 +1,4 @@
layer at (0,0) size 982x2059
layer at (0,0) size 960x2059
RenderView at (0,0) size 785x585
layer at (0,0) size 785x2059
RenderBlock {HTML} at (0,0) size 785x2059
@@ -12,7 +12,7 @@ layer at (0,0) size 785x2059
text run at (0,0) width 541: "percentage height images in DIV with no height (red) in a DIV with no height (green)"
RenderBlock {DIV} at (32,735) size 657x657 [border: (3px dotted #008000)]
RenderBlock {DIV} at (35,35) size 587x587 [border: (1px solid #FF0000)]
RenderImage {IMG} at (1,1) size 882x585
RenderImage {IMG} at (1,1) size 860x585
RenderText {#text} at (0,0) size 0x0
RenderBlock {P} at (0,1424) size 721x18
RenderText {#text} at (0,0) size 471x18
@@ -1,3 +1,56 @@
2013-06-25 David Hyatt <hyatt@apple.com>

Flex boxes (both old and new) don't handle max-height images correctly.
https://bugs.webkit.org/show_bug.cgi?id=118000

Reviewed by Beth Dakin.

Tests: css3/flexbox/image-percent-max-height.html
fast/flexbox/image-percent-max-height.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::dirtyForLayoutFromPercentageHeightDescendants):
(WebCore::RenderBlock::layoutBlockChildren):
Pull the percentage height descendant code that dirties those descendants
out of layoutBlockChildren and into a protected helper function,
dirtyForLayoutFromPercentageHeightDescendants, that can be called from the
flex box code.

Also patch dirtyForLayoutFromPercentageHeightDescendants so that it will dirty
preferred logical widths when a child has an aspect ratio, since we know that
percentage height changes will potentially affect the preferred widths of the image and
its ancestor blocks.

* rendering/RenderBlock.h:
Declaration of the new dirtyForLayoutFromPercentageHeightDescendants function.

* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
Make the old flex box code call dirtyForLayoutFromPercentageHeightDescendants so
that everything is dirtied properly.

(WebCore::RenderDeprecatedFlexibleBox::layoutHorizontalBox):
(WebCore::RenderDeprecatedFlexibleBox::layoutVerticalBox):
Remove the isReplaced()/percentage height/width dirtying now that the old flexible
box is using the same dirtying mechanism as RenderBlock.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
Patch the new flexible box code to use the dirtying mechanism that RenderBlock
uses for percentage heights/widths on replaced descendants.

* rendering/RenderObject.h:
(WebCore::RenderObject::hasAspectRatio):
Pulled the static helper function from RenderReplaced into a full-blown method
on RenderObject, so that dirtyForLayoutFromPercentageHeightDescendants can call
it to check if an object has an aspect ratio.

* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeAspectRatioInformationForRenderBox):
(WebCore::RenderReplaced::computeIntrinsicRatioInformation):
Patch the call sites of the static helper function to use hasAspectRatio instead
and get rid of the static in the cpp file.

2013-06-26 Kangil Han <kangil.han@samsung.com>

Adopt is/toHTMLAreaElement for code cleanup
@@ -2519,25 +2519,40 @@ void RenderBlock::updateBlockChildDirtyBitsBeforeLayout(bool relayoutChildren, R
child->setPreferredLogicalWidthsDirty(true, MarkOnlyThis);
}

void RenderBlock::layoutBlockChildren(bool relayoutChildren, LayoutUnit& maxFloatLogicalBottom)
void RenderBlock::dirtyForLayoutFromPercentageHeightDescendants()
{
if (gPercentHeightDescendantsMap) {
if (TrackedRendererListHashSet* descendants = gPercentHeightDescendantsMap->get(this)) {
TrackedRendererListHashSet::iterator end = descendants->end();
for (TrackedRendererListHashSet::iterator it = descendants->begin(); it != end; ++it) {
RenderBox* box = *it;
while (box != this) {
if (box->normalChildNeedsLayout())
break;
box->setChildNeedsLayout(true, MarkOnlyThis);
box = box->containingBlock();
ASSERT(box);
if (!box)
break;
}
}
if (!gPercentHeightDescendantsMap)
return;

TrackedRendererListHashSet* descendants = gPercentHeightDescendantsMap->get(this);
if (!descendants)
return;

TrackedRendererListHashSet::iterator end = descendants->end();
for (TrackedRendererListHashSet::iterator it = descendants->begin(); it != end; ++it) {
RenderBox* box = *it;
while (box != this) {
if (box->normalChildNeedsLayout())
break;
box->setChildNeedsLayout(true, MarkOnlyThis);

// If the width of an image is affected by the height of a child (e.g., an image with an aspect ratio),
// then we have to dirty preferred widths, since even enclosing blocks can become dirty as a result.
// (A horizontal flexbox that contains an inline image wrapped in an anonymous block for example.)
if (box->hasAspectRatio())
box->setPreferredLogicalWidthsDirty(true);

box = box->containingBlock();
ASSERT(box);
if (!box)
break;
}
}
}

void RenderBlock::layoutBlockChildren(bool relayoutChildren, LayoutUnit& maxFloatLogicalBottom)
{
dirtyForLayoutFromPercentageHeightDescendants();

LayoutUnit beforeEdge = borderAndPaddingBefore();
LayoutUnit afterEdge = borderAndPaddingAfter() + scrollbarLogicalHeight();
@@ -1094,6 +1094,8 @@ class RenderBlock : public RenderBox {
static void repaintDirtyFloats(Vector<FloatWithRect>& floats);

protected:
void dirtyForLayoutFromPercentageHeightDescendants();

void determineLogicalLeftPositionForChild(RenderBox* child, ApplyLayoutDeltaMode = DoNotApplyLayoutDelta);

// Pagination routines.
@@ -331,6 +331,8 @@ void RenderDeprecatedFlexibleBox::layoutBlock(bool relayoutChildren, LayoutUnit)
ChildFrameRects oldChildRects;
appendChildFrameRects(this, oldChildRects);

dirtyForLayoutFromPercentageHeightDescendants();

if (isHorizontal())
layoutHorizontalBox(relayoutChildren);
else
@@ -456,7 +458,7 @@ void RenderDeprecatedFlexibleBox::layoutHorizontalBox(bool relayoutChildren)
// our box's intrinsic height.
LayoutUnit maxAscent = 0, maxDescent = 0;
for (RenderBox* child = iterator.first(); child; child = iterator.next()) {
if (relayoutChildren || (child->isReplaced() && (child->style()->width().isPercent() || child->style()->height().isPercent())))
if (relayoutChildren)
child->setChildNeedsLayout(true, MarkOnlyThis);

if (child->isOutOfFlowPositioned())
@@ -758,7 +760,7 @@ void RenderDeprecatedFlexibleBox::layoutVerticalBox(bool relayoutChildren)
size_t childIndex = 0;
for (RenderBox* child = iterator.first(); child; child = iterator.next()) {
// Make sure we relayout children if we need it.
if (!haveLineClamp && (relayoutChildren || (child->isReplaced() && (child->style()->width().isPercent() || child->style()->height().isPercent()))))
if (!haveLineClamp && relayoutChildren)
child->setChildNeedsLayout(true, MarkOnlyThis);

if (child->isOutOfFlowPositioned()) {
@@ -349,6 +349,8 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren, LayoutUnit)

RenderBlock::startDelayUpdateScrollInfo();

dirtyForLayoutFromPercentageHeightDescendants();

Vector<LineContext> lineContexts;
OrderHashSet orderValues;
computeMainAxisPreferredSizes(orderValues);
@@ -478,6 +478,8 @@ class RenderObject : public CachedImageClient {
virtual bool isSVGResourceFilter() const { return false; }
virtual bool isSVGResourceFilterPrimitive() const { return false; }

bool hasAspectRatio() const { return isReplaced() && (isImage() || isVideo() || isCanvas()); }

virtual RenderSVGResourceContainer* toRenderSVGResourceContainer();

// FIXME: Those belong into a SVG specific base-class for all renderers (see above)
@@ -248,12 +248,6 @@ bool RenderReplaced::hasReplacedLogicalHeight() const
return false;
}

static inline bool rendererHasAspectRatio(const RenderObject* renderer)
{
ASSERT(renderer);
return renderer->isImage() || renderer->isCanvas() || renderer->isVideo();
}

void RenderReplaced::computeAspectRatioInformationForRenderBox(RenderBox* contentRenderer, FloatSize& constrainedSize, double& intrinsicRatio, bool& isPercentageIntrinsicSize) const
{
FloatSize intrinsicSize;
@@ -266,7 +260,7 @@ void RenderReplaced::computeAspectRatioInformationForRenderBox(RenderBox* conten
if (!isPercentageIntrinsicSize)
intrinsicSize.scale(style()->effectiveZoom());

if (rendererHasAspectRatio(this) && isPercentageIntrinsicSize)
if (hasAspectRatio() && isPercentageIntrinsicSize)
intrinsicRatio = 1;

// Update our intrinsic size to match what the content renderer has computed, so that when we
@@ -312,7 +306,7 @@ void RenderReplaced::computeIntrinsicRatioInformation(FloatSize& intrinsicSize,
intrinsicSize = FloatSize(intrinsicLogicalWidth(), intrinsicLogicalHeight());

// Figure out if we need to compute an intrinsic ratio.
if (intrinsicSize.isEmpty() || !rendererHasAspectRatio(this))
if (intrinsicSize.isEmpty() || !hasAspectRatio())
return;

intrinsicRatio = intrinsicSize.width() / intrinsicSize.height();

0 comments on commit 77e251c

Please sign in to comment.