- Revision
- 142931
- Author
- o...@chromium.org
- Date
- 2013-02-14 15:58:02 -0800 (Thu, 14 Feb 2013)
Log Message
Intrinsic and preferred widths on replaced elements are wrong in many cases
https://bugs.webkit.org/show_bug.cgi?id=109859
Reviewed by Levi Weintraub.
Source/WebCore:
Test: fast/replaced/preferred-widths.html
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeIntrinsicLogicalWidths):
Separate out computing the intrinsic widths. Eventually,
we should be able to share computePreferredLogicalWidth implementations
for all replaced elements and form controls since only the intrinsic width
changes.
(WebCore::RenderReplaced::computePreferredLogicalWidths):
-Apply min-width and max-width constraints and then add borderAndPaddingLogicalWidth
at the end to make sure it's always applied. This matches all our other
computePreferredLogicalWidths override and makes use match Gecko's/Opera's rendering.
-Only set the minPreferredLogicalWidth to 0 if the width or max-width is a percent value.
Doing it for height values and for min-width doesn't make any sense and doesn't
match other browsers. Doing this for max-width still doesn't match other browsers,
but it sounds like Gecko at least would like to change that.
* rendering/RenderReplaced.h:
(WebCore::RenderReplaced::hasRelativeIntrinsicLogicalWidth):
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::hasRelativeIntrinsicLogicalWidth):
Add a way to check if the logicalWidth is relative so that we only check
the width in computePreferredLogicalWidths instead of also checking the height.
* rendering/svg/RenderSVGRoot.h:
LayoutTests:
* fast/replaced/preferred-widths-expected.txt: Added.
* fast/replaced/preferred-widths.html: Added.
These results match Gecko and Opera except for the 3rd container div.
Talking to dbaron and bz and Mozilla they sound likely to match our behavior there.
See https://bugzilla.mozilla.org/show_bug.cgi?id=823483 for example.
The width of the containers is wrong in some of these cases because our
computePreferredLogicalWidths methods don't currently account for
intrinsic sizes (e.g. min-content, max-content, etc).
* platform/chromium-linux/fast/replaced/width100percent-image-expected.png:
* platform/chromium-linux/tables/mozilla_expected_failures/bugs/bug85016-expected.png:
* platform/chromium-win/fast/replaced/width100percent-image-expected.txt:
* platform/chromium-win/tables/mozilla_expected_failures/bugs/bug85016-expected.txt:
These new results are more correct. The width100percent-image case now
matches other browsers and is due to not setting the minPreferrredLogicalWidth to
0 if the height is a percentage. The bugs85016 case is different because we
now correctly add the border and padding width to the preferred width of the image.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (142930 => 142931)
--- trunk/LayoutTests/ChangeLog 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/LayoutTests/ChangeLog 2013-02-14 23:58:02 UTC (rev 142931)
@@ -1,3 +1,29 @@
+2013-02-14 Ojan Vafai <o...@chromium.org>
+
+ Intrinsic and preferred widths on replaced elements are wrong in many cases
+ https://bugs.webkit.org/show_bug.cgi?id=109859
+
+ Reviewed by Levi Weintraub.
+
+ * fast/replaced/preferred-widths-expected.txt: Added.
+ * fast/replaced/preferred-widths.html: Added.
+ These results match Gecko and Opera except for the 3rd container div.
+ Talking to dbaron and bz and Mozilla they sound likely to match our behavior there.
+ See https://bugzilla.mozilla.org/show_bug.cgi?id=823483 for example.
+
+ The width of the containers is wrong in some of these cases because our
+ computePreferredLogicalWidths methods don't currently account for
+ intrinsic sizes (e.g. min-content, max-content, etc).
+
+ * platform/chromium-linux/fast/replaced/width100percent-image-expected.png:
+ * platform/chromium-linux/tables/mozilla_expected_failures/bugs/bug85016-expected.png:
+ * platform/chromium-win/fast/replaced/width100percent-image-expected.txt:
+ * platform/chromium-win/tables/mozilla_expected_failures/bugs/bug85016-expected.txt:
+ These new results are more correct. The width100percent-image case now
+ matches other browsers and is due to not setting the minPreferrredLogicalWidth to
+ 0 if the height is a percentage. The bugs85016 case is different because we
+ now correctly add the border and padding width to the preferred width of the image.
+
2013-02-14 Ryosuke Niwa <rn...@webkit.org>
Add a crash test expectation to media/media-captions.html on Mac per bug 109869.
Added: trunk/LayoutTests/fast/replaced/preferred-widths-expected.txt (0 => 142931)
--- trunk/LayoutTests/fast/replaced/preferred-widths-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/replaced/preferred-widths-expected.txt 2013-02-14 23:58:02 UTC (rev 142931)
@@ -0,0 +1,31 @@
+Test the effect of percentages widths on the preferred widths of replaced elements.
+
+
+PASS
+
+PASS
+
+FAIL:
+Expected 130 for width, but got 30.
+
+<div class="container" data-expected-width="130" data-expected-height="130">
+ <img class="min-content" src="" style="width: 100%; border: 5px solid black; padding: 5px;" data-expected-width="120" data-expected-height="120">
+</div>
+
+FAIL:
+Expected 130 for width, but got 30.
+
+<div class="container" data-expected-width="130" data-expected-height="130">
+ <img class="max-content" src="" style="width: 100%; border: 5px solid black; padding: 5px;" data-expected-width="120" data-expected-height="120">
+</div>
+
+PASS
+
+PASS
+
+PASS
+
+PASS
+
+PASS
+
Added: trunk/LayoutTests/fast/replaced/preferred-widths.html (0 => 142931)
--- trunk/LayoutTests/fast/replaced/preferred-widths.html (rev 0)
+++ trunk/LayoutTests/fast/replaced/preferred-widths.html 2013-02-14 23:58:02 UTC (rev 142931)
@@ -0,0 +1,64 @@
+<style>
+.container {
+ display: inline-block;
+ border: 5px solid salmon;
+}
+.min-content {
+ min-width: -webkit-min-content;
+ min-width: -moz-min-content;
+ min-width: -ie-min-content;
+ min-width: min-content;
+}
+.max-content {
+ min-width: -webkit-max-content;
+ min-width: -moz-max-content;
+ min-width: -ie-max-content;
+ min-width: max-content;
+}
+</style>
+<p>Test the effect of percentages widths on the preferred widths of replaced elements.</p>
+
+<div style="width: 0; position: relative">
+
+<div class="container" data-expected-width=130 data-expected-height=130>
+ <img class="min-content" src="" style="width: 10px; border: 5px solid black; padding: 5px;" data-expected-width=120 data-expected-height=120>
+</div>
+
+<div class="container" data-expected-width=130 data-expected-height=130>
+ <img class="max-content" src="" style="width: 10px; border: 5px solid black; padding: 5px;" data-expected-width=120 data-expected-height=120>
+</div>
+
+<div class="container" data-expected-width=130 data-expected-height=130>
+ <img class="min-content" src="" style="width: 100%; border: 5px solid black; padding: 5px;" data-expected-width=120 data-expected-height=120>
+</div>
+
+<div class="container" data-expected-width=130 data-expected-height=130>
+ <img class="max-content" src="" style="width: 100%; border: 5px solid black; padding: 5px;" data-expected-width=120 data-expected-height=120>
+</div>
+
+<div class="container" data-expected-width=55 data-expected-height=75>
+ <img src="" style="min-width: 25px; width: 100%; border: 5px solid black; padding: 5px;" data-expected-width=65 data-expected-height=65>
+</div>
+
+<div class="container" data-expected-width=55 data-expected-height=75>
+ <img src="" style="min-width: 25px; width: 100%; border: 5px solid black; padding: 5px;" data-expected-width=65 data-expected-height=65>
+</div>
+
+<div class="container" data-expected-width=30 data-expected-height=50>
+ <img src="" style="width: 100%; border: 5px solid grey; padding: 5px;" data-expected-width=40 data-expected-height=40>
+</div>
+
+<div class="container" data-expected-width=30 data-expected-height=50>
+ <img src="" style="max-width: 100%; border: 5px solid grey; padding: 5px;" data-expected-width=40 data-expected-height=40>
+</div>
+
+<div class="container" data-expected-width=130 data-expected-height=150>
+ <img src="" style="min-width: 100%; border: 5px solid grey; padding: 5px;" data-expected-width=140 data-expected-height=140>
+</div>
+
+</div>
+
+<script src=""
+<script>
+checkLayout(".container");
+</script>
\ No newline at end of file
Modified: trunk/LayoutTests/platform/chromium-linux/fast/replaced/width100percent-image-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-linux/tables/mozilla_expected_failures/bugs/bug85016-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-win/fast/replaced/width100percent-image-expected.txt (142930 => 142931)
--- trunk/LayoutTests/platform/chromium-win/fast/replaced/width100percent-image-expected.txt 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/LayoutTests/platform/chromium-win/fast/replaced/width100percent-image-expected.txt 2013-02-14 23:58:02 UTC (rev 142931)
@@ -28,10 +28,10 @@
RenderTableRow {TR} at (0,1) size 769x277
RenderTableCell {TD} at (1,1) size 216x277 [r=0 c=0 rs=1 cs=1]
RenderImage {IMG} at (1,1) size 214x275
- RenderTableCell {TD} at (218,1) size 2x277 [r=0 c=1 rs=1 cs=1]
+ RenderTableCell {TD} at (218,1) size 216x277 [r=0 c=1 rs=1 cs=1]
RenderImage {IMG} at (1,1) size 214x275
- RenderTableCell {TD} at (221,1) size 2x277 [r=0 c=2 rs=1 cs=1]
+ RenderTableCell {TD} at (435,1) size 216x277 [r=0 c=2 rs=1 cs=1]
RenderImage {IMG} at (1,1) size 214x275
- RenderTableCell {TD} at (224,128) size 544x22 [r=0 c=3 rs=1 cs=1]
+ RenderTableCell {TD} at (652,128) size 116x22 [r=0 c=3 rs=1 cs=1]
RenderText {#text} at (1,1) size 4x19
text run at (1,1) width 4: " "
Modified: trunk/LayoutTests/platform/chromium-win/tables/mozilla_expected_failures/bugs/bug85016-expected.txt (142930 => 142931)
--- trunk/LayoutTests/platform/chromium-win/tables/mozilla_expected_failures/bugs/bug85016-expected.txt 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/LayoutTests/platform/chromium-win/tables/mozilla_expected_failures/bugs/bug85016-expected.txt 2013-02-14 23:58:02 UTC (rev 142931)
@@ -1,4 +1,4 @@
-layer at (0,0) size 960x2063
+layer at (0,0) size 982x2063
RenderView at (0,0) size 785x585
layer at (0,0) size 785x2063
RenderBlock {HTML} at (0,0) size 785x2063
@@ -12,7 +12,7 @@
text run at (0,0) width 506: "percentage height images in DIV with no height (red) in a DIV with no height (green)"
RenderBlock {DIV} at (32,737) size 657x657 [border: (3px dotted #008000)]
RenderBlock {DIV} at (35,35) size 587x587 [border: (1px solid #FF0000)]
- RenderImage {IMG} at (1,1) size 860x585
+ RenderImage {IMG} at (1,1) size 882x585
RenderText {#text} at (0,0) size 0x0
RenderBlock {P} at (0,1426) size 721x20
RenderText {#text} at (0,0) size 443x19
Modified: trunk/Source/WebCore/ChangeLog (142930 => 142931)
--- trunk/Source/WebCore/ChangeLog 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/Source/WebCore/ChangeLog 2013-02-14 23:58:02 UTC (rev 142931)
@@ -1,3 +1,37 @@
+2013-02-14 Ojan Vafai <o...@chromium.org>
+
+ Intrinsic and preferred widths on replaced elements are wrong in many cases
+ https://bugs.webkit.org/show_bug.cgi?id=109859
+
+ Reviewed by Levi Weintraub.
+
+ Test: fast/replaced/preferred-widths.html
+
+ * rendering/RenderReplaced.cpp:
+ (WebCore::RenderReplaced::computeIntrinsicLogicalWidths):
+ Separate out computing the intrinsic widths. Eventually,
+ we should be able to share computePreferredLogicalWidth implementations
+ for all replaced elements and form controls since only the intrinsic width
+ changes.
+
+ (WebCore::RenderReplaced::computePreferredLogicalWidths):
+ -Apply min-width and max-width constraints and then add borderAndPaddingLogicalWidth
+ at the end to make sure it's always applied. This matches all our other
+ computePreferredLogicalWidths override and makes use match Gecko's/Opera's rendering.
+ -Only set the minPreferredLogicalWidth to 0 if the width or max-width is a percent value.
+ Doing it for height values and for min-width doesn't make any sense and doesn't
+ match other browsers. Doing this for max-width still doesn't match other browsers,
+ but it sounds like Gecko at least would like to change that.
+
+ * rendering/RenderReplaced.h:
+ (WebCore::RenderReplaced::hasRelativeIntrinsicLogicalWidth):
+ * rendering/svg/RenderSVGRoot.cpp:
+ (WebCore::RenderSVGRoot::hasRelativeIntrinsicLogicalWidth):
+ Add a way to check if the logicalWidth is relative so that we only check
+ the width in computePreferredLogicalWidths instead of also checking the height.
+
+ * rendering/svg/RenderSVGRoot.h:
+
2013-02-14 Stephen Chenney <schen...@chromium.org>
Crash when selecting a HarfBuzz text run with SVG fonts included
Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (142930 => 142931)
--- trunk/Source/WebCore/rendering/RenderReplaced.cpp 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp 2013-02-14 23:58:02 UTC (rev 142931)
@@ -439,33 +439,37 @@
return computeReplacedLogicalHeightRespectingMinMaxHeight(intrinsicLogicalHeight());
}
-LayoutUnit RenderReplaced::computeMaxPreferredLogicalWidth() const
+void RenderReplaced::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
{
- Length logicalWidth = style()->logicalWidth();
-
// We cannot resolve any percent logical width here as the available logical
// width may not be set on our containing block.
- if (logicalWidth.isPercent())
- return intrinsicLogicalWidth();
-
- return computeReplacedLogicalWidth(ComputePreferred);
+ minLogicalWidth = maxLogicalWidth = style()->logicalWidth().isPercent() ? intrinsicLogicalWidth() : computeReplacedLogicalWidth(ComputePreferred);
}
void RenderReplaced::computePreferredLogicalWidths()
{
ASSERT(preferredLogicalWidthsDirty());
- LayoutUnit borderAndPadding = borderAndPaddingLogicalWidth();
- m_maxPreferredLogicalWidth = computeMaxPreferredLogicalWidth() + borderAndPadding;
+ computeIntrinsicLogicalWidths(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
- if (style()->maxWidth().isFixed())
- m_maxPreferredLogicalWidth = min<LayoutUnit>(m_maxPreferredLogicalWidth, style()->maxWidth().value() + (style()->boxSizing() == CONTENT_BOX ? borderAndPadding : LayoutUnit()));
-
- if (hasRelativeDimensions())
+ RenderStyle* styleToUse = style();
+ if (styleToUse->logicalWidth().isPercent() || styleToUse->logicalMaxWidth().isPercent() || hasRelativeIntrinsicLogicalWidth())
m_minPreferredLogicalWidth = 0;
- else
- m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
+ if (styleToUse->logicalMinWidth().isFixed() && styleToUse->logicalMinWidth().value() > 0) {
+ m_maxPreferredLogicalWidth = max(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse->logicalMinWidth().value()));
+ m_minPreferredLogicalWidth = max(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse->logicalMinWidth().value()));
+ }
+
+ if (styleToUse->logicalMaxWidth().isFixed()) {
+ m_maxPreferredLogicalWidth = min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse->logicalMaxWidth().value()));
+ m_minPreferredLogicalWidth = min(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse->logicalMaxWidth().value()));
+ }
+
+ LayoutUnit borderAndPadding = borderAndPaddingLogicalWidth();
+ m_minPreferredLogicalWidth += borderAndPadding;
+ m_maxPreferredLogicalWidth += borderAndPadding;
+
setPreferredLogicalWidthsDirty(false);
}
Modified: trunk/Source/WebCore/rendering/RenderReplaced.h (142930 => 142931)
--- trunk/Source/WebCore/rendering/RenderReplaced.h 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/Source/WebCore/rendering/RenderReplaced.h 2013-02-14 23:58:02 UTC (rev 142931)
@@ -48,6 +48,8 @@
virtual LayoutSize intrinsicSize() const OVERRIDE { return m_intrinsicSize; }
virtual void computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio, bool& isPercentageIntrinsicSize) const;
+ virtual void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const OVERRIDE;
+
virtual LayoutUnit minimumReplacedHeight() const { return LayoutUnit(); }
virtual void setSelectionState(SelectionState);
@@ -58,6 +60,7 @@
void setIntrinsicSize(const LayoutSize& intrinsicSize) { m_intrinsicSize = intrinsicSize; }
virtual void intrinsicSizeChanged();
+ virtual bool hasRelativeIntrinsicLogicalWidth() const { return false; }
virtual void paint(PaintInfo&, const LayoutPoint&);
bool shouldPaint(PaintInfo&, const LayoutPoint&);
@@ -69,7 +72,6 @@
virtual bool canHaveChildren() const { return false; }
- LayoutUnit computeMaxPreferredLogicalWidth() const;
virtual void computePreferredLogicalWidths();
virtual void paintReplaced(PaintInfo&, const LayoutPoint&) { }
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (142930 => 142931)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2013-02-14 23:58:02 UTC (rev 142931)
@@ -469,6 +469,13 @@
return svg->intrinsicHeight(SVGSVGElement::IgnoreCSSProperties).isPercent() || svg->intrinsicWidth(SVGSVGElement::IgnoreCSSProperties).isPercent();
}
+bool RenderSVGRoot::hasRelativeIntrinsicLogicalWidth() const
+{
+ SVGSVGElement* svg = static_cast<SVGSVGElement*>(node());
+ ASSERT(svg);
+ return svg->intrinsicWidth(SVGSVGElement::IgnoreCSSProperties).isPercent();
+}
+
bool RenderSVGRoot::hasRelativeLogicalHeight() const
{
SVGSVGElement* svg = static_cast<SVGSVGElement*>(node());
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h (142930 => 142931)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h 2013-02-14 23:51:45 UTC (rev 142930)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h 2013-02-14 23:58:02 UTC (rev 142931)
@@ -58,8 +58,9 @@
IntSize containerSize() const { return m_containerSize; }
void setContainerSize(const IntSize& containerSize) { m_containerSize = containerSize; }
- virtual bool hasRelativeDimensions() const;
- virtual bool hasRelativeLogicalHeight() const;
+ virtual bool hasRelativeDimensions() const OVERRIDE;
+ virtual bool hasRelativeIntrinsicLogicalWidth() const OVERRIDE;
+ virtual bool hasRelativeLogicalHeight() const OVERRIDE;
// localToBorderBoxTransform maps local SVG viewport coordinates to local CSS box coordinates.
const AffineTransform& localToBorderBoxTransform() const { return m_localToBorderBoxTransform; }
