Percentage min/max width replaced element may incorrectly rendered
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2012 01:52:05 +0000 (01:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2012 01:52:05 +0000 (01:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105264

Patch by KyungTae Kim <ktf.kim@samsung.com> on 2012-12-20
Reviewed by Tony Chang.

Source/WebCore:

To make do not include percentage min width in preferred logical width calculation,
because we cannot resolve it for preferred width.

Test: fast/css/percent-min-width-img-src-change.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computeReplacedLogicalWidth):
Modify includeMaxWidth parameter to shouldComputePreferred.
(WebCore::RenderBox::computeReplacedLogicalWidthRespectingMinMaxWidth):
Modify includeMaxWidth parameter to shouldComputePreferred.
If shouldComputePreferred is ComputePreferred,
don't use minLogicalWidth or maxLogicalWidth if they are percent type.
* rendering/RenderBox.h:
(RenderBox):
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeReplacedLogicalWidth):
Modify includeMaxWidth parameter to shouldComputePreferred.
(WebCore::RenderReplaced::computeMaxPreferredLogicalWidth):
Modify from set includeMaxWidth=false to set shouldComputePreferred=ComputePreferred.
* rendering/RenderReplaced.h:
(RenderReplaced):
* rendering/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::computeReplacedLogicalWidth):
Modify includeMaxWidth parameter to shouldComputePreferred.
* rendering/RenderSVGRoot.h:
(RenderSVGRoot):
* rendering/RenderVideo.cpp:
(WebCore::RenderVideo::computeReplacedLogicalWidth):
Modify includeMaxWidth parameter to shouldComputePreferred.
* rendering/RenderVideo.h:
(RenderVideo):

LayoutTests:

Add test to check when the source of images with percentage min-width is changed.

* fast/css/percent-min-width-img-src-change-expected.txt: Added.
* fast/css/percent-min-width-img-src-change.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@138332 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/percent-min-width-img-src-change-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/percent-min-width-img-src-change.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderReplaced.cpp
Source/WebCore/rendering/RenderReplaced.h
Source/WebCore/rendering/RenderVideo.cpp
Source/WebCore/rendering/RenderVideo.h
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/RenderSVGRoot.h

index 3f67b50..52efb7d 100644 (file)
@@ -1,3 +1,15 @@
+2012-12-20  KyungTae Kim  <ktf.kim@samsung.com>
+
+        Percentage min/max width replaced element may incorrectly rendered
+        https://bugs.webkit.org/show_bug.cgi?id=105264
+
+        Reviewed by Tony Chang.
+
+        Add test to check when the source of images with percentage min-width is changed.
+
+        * fast/css/percent-min-width-img-src-change-expected.txt: Added.
+        * fast/css/percent-min-width-img-src-change.html: Added.
+
 2012-12-20  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add Mac test expectations for the bug 73865.
diff --git a/LayoutTests/fast/css/percent-min-width-img-src-change-expected.txt b/LayoutTests/fast/css/percent-min-width-img-src-change-expected.txt
new file mode 100644 (file)
index 0000000..a700cb6
--- /dev/null
@@ -0,0 +1,2 @@
+PASS
diff --git a/LayoutTests/fast/css/percent-min-width-img-src-change.html b/LayoutTests/fast/css/percent-min-width-img-src-change.html
new file mode 100644 (file)
index 0000000..b001a9b
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script type="text/javascript">
+var completed = 0, failures = 0, failuresDetail = ""; 
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+function printResult() {
+    document.getElementById("result").innerText = failures ? "FAIL: " + failures + " cases failed\n" + failuresDetail : "PASS";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+
+function imageCheckForMin(image) {
+    if (image.src.match("greenbox-100px.png")  != null) {
+        image.src = "resources/greenbox.png";
+    }
+    else {
+        ++completed;
+        if (image.width != 25) {
+            ++failures; 
+            failuresDetail +=  image.id + ": " + image.width + " (expected: " + 25 + ")\n";
+        }
+        if (completed == 2)
+             printResult();
+    }
+}
+</script>
+</head>
+
+<body>
+<div id="result"></div>
+<table>
+<tbody>
+<tr>
+<td>
+<img id="percentMinWidthInTable" style="min-width:100%;" src="resources/greenbox-100px.png" onload="imageCheckForMin(this)">
+</td>
+</tr>
+</tbody>
+</table>
+<div style="position:absolute;top:200px;left:11px">
+<img id="percentMinWidthInAbsolute" style="min-width:100%;" src="resources/greenbox-100px.png" onload="imageCheckForMin(this)">
+</div>
+</body>
+</html>
index 3d88236..0d61365 100644 (file)
@@ -1,3 +1,42 @@
+2012-12-20  KyungTae Kim  <ktf.kim@samsung.com>
+
+        Percentage min/max width replaced element may incorrectly rendered
+        https://bugs.webkit.org/show_bug.cgi?id=105264
+
+        Reviewed by Tony Chang.
+
+        To make do not include percentage min width in preferred logical width calculation,
+        because we cannot resolve it for preferred width.
+
+        Test: fast/css/percent-min-width-img-src-change.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeReplacedLogicalWidth):
+        Modify includeMaxWidth parameter to shouldComputePreferred.
+        (WebCore::RenderBox::computeReplacedLogicalWidthRespectingMinMaxWidth):
+        Modify includeMaxWidth parameter to shouldComputePreferred.
+        If shouldComputePreferred is ComputePreferred, 
+        don't use minLogicalWidth or maxLogicalWidth if they are percent type.
+        * rendering/RenderBox.h:
+        (RenderBox):
+        * rendering/RenderReplaced.cpp:
+        (WebCore::RenderReplaced::computeReplacedLogicalWidth):
+        Modify includeMaxWidth parameter to shouldComputePreferred.
+        (WebCore::RenderReplaced::computeMaxPreferredLogicalWidth):
+        Modify from set includeMaxWidth=false to set shouldComputePreferred=ComputePreferred.
+        * rendering/RenderReplaced.h:
+        (RenderReplaced):
+        * rendering/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::computeReplacedLogicalWidth):
+        Modify includeMaxWidth parameter to shouldComputePreferred.
+        * rendering/RenderSVGRoot.h:
+        (RenderSVGRoot):
+        * rendering/RenderVideo.cpp:
+        (WebCore::RenderVideo::computeReplacedLogicalWidth):
+        Modify includeMaxWidth parameter to shouldComputePreferred.
+        * rendering/RenderVideo.h:
+        (RenderVideo):
+
 2012-12-20  Alexey Proskuryakov  <ap@apple.com>
 
         REGRESSION (r138191): Tests crash in ResourceRequest::setStorageSession
index 92bc53b..58a745a 100644 (file)
@@ -2415,15 +2415,15 @@ LayoutUnit RenderBox::computePercentageLogicalHeight(const Length& height) const
     return result;
 }
 
-LayoutUnit RenderBox::computeReplacedLogicalWidth(bool includeMaxWidth) const
+LayoutUnit RenderBox::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
-    return computeReplacedLogicalWidthRespectingMinMaxWidth(computeReplacedLogicalWidthUsing(MainOrPreferredSize, style()->logicalWidth()), includeMaxWidth);
+    return computeReplacedLogicalWidthRespectingMinMaxWidth(computeReplacedLogicalWidthUsing(MainOrPreferredSize, style()->logicalWidth()), shouldComputePreferred);
 }
 
-LayoutUnit RenderBox::computeReplacedLogicalWidthRespectingMinMaxWidth(LayoutUnit logicalWidth, bool includeMaxWidth) const
+LayoutUnit RenderBox::computeReplacedLogicalWidthRespectingMinMaxWidth(LayoutUnit logicalWidth, ShouldComputePreferred shouldComputePreferred) const
 {
-    LayoutUnit minLogicalWidth = computeReplacedLogicalWidthUsing(MinSize, style()->logicalMinWidth());
-    LayoutUnit maxLogicalWidth = !includeMaxWidth || style()->logicalMaxWidth().isUndefined() ? logicalWidth : computeReplacedLogicalWidthUsing(MaxSize, style()->logicalMaxWidth());
+    LayoutUnit minLogicalWidth = (shouldComputePreferred == ComputePreferred && style()->logicalMinWidth().isPercent()) || style()->logicalMinWidth().isUndefined() ? logicalWidth : computeReplacedLogicalWidthUsing(MinSize, style()->logicalMinWidth());
+    LayoutUnit maxLogicalWidth = (shouldComputePreferred == ComputePreferred && style()->logicalMaxWidth().isPercent()) || style()->logicalMaxWidth().isUndefined() ? logicalWidth : computeReplacedLogicalWidthUsing(MaxSize, style()->logicalMaxWidth());
     return max(minLogicalWidth, min(logicalWidth, maxLogicalWidth));
 }
 
index ec3bd6d..869f36c 100644 (file)
@@ -40,6 +40,8 @@ enum SizeType { MainOrPreferredSize, MinSize, MaxSize };
 
 enum OverlayScrollbarSizeRelevancy { IgnoreOverlayScrollbarSize, IncludeOverlayScrollbarSize };
 
+enum ShouldComputePreferred { ComputeActual, ComputePreferred };
+
 class RenderBox : public RenderBoxModelObject {
 public:
     RenderBox(Node*);
@@ -414,11 +416,11 @@ public:
     LayoutUnit computeContentLogicalHeight(SizeType, const Length& height);
     LayoutUnit computeContentAndScrollbarLogicalHeightUsing(SizeType, const Length& height) const;
     LayoutUnit computeReplacedLogicalWidthUsing(SizeType, Length width) const;
-    LayoutUnit computeReplacedLogicalWidthRespectingMinMaxWidth(LayoutUnit logicalWidth, bool includeMaxWidth = true) const;
+    LayoutUnit computeReplacedLogicalWidthRespectingMinMaxWidth(LayoutUnit logicalWidth, ShouldComputePreferred  = ComputeActual) const;
     LayoutUnit computeReplacedLogicalHeightUsing(SizeType, Length height) const;
     LayoutUnit computeReplacedLogicalHeightRespectingMinMaxHeight(LayoutUnit logicalHeight) const;
 
-    virtual LayoutUnit computeReplacedLogicalWidth(bool includeMaxWidth = true) const;
+    virtual LayoutUnit computeReplacedLogicalWidth(ShouldComputePreferred  = ComputeActual) const;
     virtual LayoutUnit computeReplacedLogicalHeight() const;
 
     static bool percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool outOfFlowPositioned);
index 623e2f0..2a4e0c4 100644 (file)
@@ -337,10 +337,10 @@ void RenderReplaced::computeIntrinsicRatioInformation(FloatSize& intrinsicSize,
     intrinsicRatio = intrinsicSize.width() / intrinsicSize.height();
 }
 
-LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) const
+LayoutUnit RenderReplaced::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
     if (style()->logicalWidth().isSpecified())
-        return computeReplacedLogicalWidthRespectingMinMaxWidth(computeReplacedLogicalWidthUsing(MainOrPreferredSize, style()->logicalWidth()), includeMaxWidth);
+        return computeReplacedLogicalWidthRespectingMinMaxWidth(computeReplacedLogicalWidthUsing(MainOrPreferredSize, style()->logicalWidth()), shouldComputePreferred);
 
     RenderBox* contentRenderer = embeddedContentBox();
 
@@ -356,7 +356,7 @@ LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) con
 
         // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, then that intrinsic width is the used value of 'width'.
         if (heightIsAuto && hasIntrinsicWidth)
-            return computeReplacedLogicalWidthRespectingMinMaxWidth(constrainedSize.width(), includeMaxWidth);
+            return computeReplacedLogicalWidthRespectingMinMaxWidth(constrainedSize.width(), shouldComputePreferred);
 
         bool hasIntrinsicHeight = !isPercentageIntrinsicSize && constrainedSize.height() > 0;
         if (intrinsicRatio || isPercentageIntrinsicSize) {
@@ -365,7 +365,7 @@ LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) con
             // of 'width' is: (used height) * (intrinsic ratio)
             if (intrinsicRatio && ((heightIsAuto && !hasIntrinsicWidth && hasIntrinsicHeight) || !heightIsAuto)) {
                 LayoutUnit logicalHeight = computeReplacedLogicalHeight();
-                return computeReplacedLogicalWidthRespectingMinMaxWidth(roundToInt(round(logicalHeight * intrinsicRatio)));
+                return computeReplacedLogicalWidthRespectingMinMaxWidth(roundToInt(round(logicalHeight * intrinsicRatio)), shouldComputePreferred);
             }
 
             // If 'height' and 'width' both have computed values of 'auto' and the element has an intrinsic ratio but no intrinsic height or width, then the used value of
@@ -376,7 +376,7 @@ LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) con
                 // 'margin-left' + 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + 'margin-right' = width of containing block
                 LayoutUnit logicalWidth;
                 if (RenderBlock* blockWithWidth = firstContainingBlockWithLogicalWidth(this))
-                    logicalWidth = blockWithWidth->computeReplacedLogicalWidthRespectingMinMaxWidth(blockWithWidth->computeReplacedLogicalWidthUsing(MainOrPreferredSize, blockWithWidth->style()->logicalWidth()), false);
+                    logicalWidth = blockWithWidth->computeReplacedLogicalWidthRespectingMinMaxWidth(blockWithWidth->computeReplacedLogicalWidthUsing(MainOrPreferredSize, blockWithWidth->style()->logicalWidth()), shouldComputePreferred);
                 else
                     logicalWidth = containingBlock()->availableLogicalWidth();
 
@@ -386,13 +386,13 @@ LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) con
                 logicalWidth = max<LayoutUnit>(0, logicalWidth - (marginStart + marginEnd + (width() - clientWidth())));
                 if (isPercentageIntrinsicSize)
                     logicalWidth = logicalWidth * constrainedSize.width() / 100;
-                return computeReplacedLogicalWidthRespectingMinMaxWidth(logicalWidth, includeMaxWidth);
+                return computeReplacedLogicalWidthRespectingMinMaxWidth(logicalWidth, shouldComputePreferred);
             }
         }
 
         // Otherwise, if 'width' has a computed value of 'auto', and the element has an intrinsic width, then that intrinsic width is the used value of 'width'.
         if (hasIntrinsicWidth)
-            return computeReplacedLogicalWidthRespectingMinMaxWidth(constrainedSize.width(), includeMaxWidth);
+            return computeReplacedLogicalWidthRespectingMinMaxWidth(constrainedSize.width(), shouldComputePreferred);
 
         // Otherwise, if 'width' has a computed value of 'auto', but none of the conditions above are met, then the used value of 'width' becomes 300px. If 300px is too
         // wide to fit the device, UAs should use the width of the largest rectangle that has a 2:1 ratio and fits the device instead.
@@ -401,7 +401,7 @@ LayoutUnit RenderReplaced::computeReplacedLogicalWidth(bool includeMaxWidth) con
         // has no intrinsic size, which is wrong per CSS 2.1, but matches our behavior since a long time.
     }
 
-    return computeReplacedLogicalWidthRespectingMinMaxWidth(intrinsicLogicalWidth(), includeMaxWidth);
+    return computeReplacedLogicalWidthRespectingMinMaxWidth(intrinsicLogicalWidth(), shouldComputePreferred);
 }
 
 LayoutUnit RenderReplaced::computeReplacedLogicalHeight() const
@@ -448,9 +448,7 @@ LayoutUnit RenderReplaced::computeMaxPreferredLogicalWidth() const
     if (logicalWidth.isPercent())
         return intrinsicLogicalWidth();
 
-    // FIXME: We shouldn't be calling a logical width computing function in preferred
-    // logical widths computation as the layout information is probably invalid.
-    return computeReplacedLogicalWidth(false);
+    return computeReplacedLogicalWidth(ComputePreferred);
 }
 
 void RenderReplaced::computePreferredLogicalWidths()
index c698616..5c64549 100644 (file)
@@ -32,7 +32,7 @@ public:
     RenderReplaced(Node*, const LayoutSize& intrinsicSize);
     virtual ~RenderReplaced();
 
-    virtual LayoutUnit computeReplacedLogicalWidth(bool includeMaxWidth = true) const;
+    virtual LayoutUnit computeReplacedLogicalWidth(ShouldComputePreferred  = ComputeActual) const OVERRIDE;
     virtual LayoutUnit computeReplacedLogicalHeight() const;
 
     bool hasReplacedLogicalWidth() const;
index 11269e2..ae3fe59 100644 (file)
@@ -262,9 +262,9 @@ void RenderVideo::updatePlayer()
     mediaPlayer->setVisible(true);
 }
 
-LayoutUnit RenderVideo::computeReplacedLogicalWidth(bool includeMaxWidth) const
+LayoutUnit RenderVideo::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
-    return RenderReplaced::computeReplacedLogicalWidth(includeMaxWidth);
+    return RenderReplaced::computeReplacedLogicalWidth(shouldComputePreferred);
 }
 
 LayoutUnit RenderVideo::computeReplacedLogicalHeight() const
index 78f8550..d6efe93 100644 (file)
@@ -70,7 +70,7 @@ private:
 
     virtual void layout();
 
-    virtual LayoutUnit computeReplacedLogicalWidth(bool includeMaxWidth = true) const;
+    virtual LayoutUnit computeReplacedLogicalWidth(ShouldComputePreferred  = ComputeActual) const OVERRIDE;
     virtual LayoutUnit computeReplacedLogicalHeight() const;
     virtual LayoutUnit minimumReplacedHeight() const OVERRIDE;
 
index 048c4f6..39fb26f 100644 (file)
@@ -159,7 +159,7 @@ static inline LayoutUnit resolveLengthAttributeForSVG(const Length& length, floa
     return static_cast<LayoutUnit>(valueForLength(length, maxSize, renderView) * (length.isFixed() ? scale : 1));
 }
 
-LayoutUnit RenderSVGRoot::computeReplacedLogicalWidth(bool includeMaxWidth) const
+LayoutUnit RenderSVGRoot::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
     SVGSVGElement* svg = static_cast<SVGSVGElement*>(node());
     ASSERT(svg);
@@ -169,7 +169,7 @@ LayoutUnit RenderSVGRoot::computeReplacedLogicalWidth(bool includeMaxWidth) cons
         return m_containerSize.width();
 
     if (style()->logicalWidth().isSpecified() || style()->logicalMaxWidth().isSpecified())
-        return RenderReplaced::computeReplacedLogicalWidth(includeMaxWidth);
+        return RenderReplaced::computeReplacedLogicalWidth(shouldComputePreferred);
 
     if (svg->widthAttributeEstablishesViewport())
         return resolveLengthAttributeForSVG(svg->intrinsicWidth(SVGSVGElement::IgnoreCSSProperties), style()->effectiveZoom(), containingBlock()->availableLogicalWidth(), view());
@@ -179,7 +179,7 @@ LayoutUnit RenderSVGRoot::computeReplacedLogicalWidth(bool includeMaxWidth) cons
         return document()->frame()->ownerRenderer()->availableLogicalWidth();
 
     // SVG embedded via SVGImage (background-image/border-image/etc) / Inline SVG.
-    return RenderReplaced::computeReplacedLogicalWidth(includeMaxWidth);
+    return RenderReplaced::computeReplacedLogicalWidth(shouldComputePreferred);
 }
 
 LayoutUnit RenderSVGRoot::computeReplacedLogicalHeight() const
index 5234ef1..35d93aa 100644 (file)
@@ -78,7 +78,7 @@ private:
     virtual bool isSVGRoot() const { return true; }
     virtual const char* renderName() const { return "RenderSVGRoot"; }
 
-    virtual LayoutUnit computeReplacedLogicalWidth(bool includeMaxWidth = true) const;
+    virtual LayoutUnit computeReplacedLogicalWidth(ShouldComputePreferred  = ComputeActual) const OVERRIDE;
     virtual LayoutUnit computeReplacedLogicalHeight() const;
     virtual void layout();
     virtual void paintReplaced(PaintInfo&, const LayoutPoint&);