- Revision
- 123183
- Author
- [email protected]
- Date
- 2012-07-19 23:12:54 -0700 (Thu, 19 Jul 2012)
Log Message
SVG not properly respecting max-width.
https://bugs.webkit.org/show_bug.cgi?id=91474
Source/WebCore:
My previous checkin for 91474 accidentally inverted width and height in the division case for
the computation of height. When I fixed this inversion, I discovered that <object> elements
are in fact also broken with max-width handling, and that furthermore, trying to apply the same
max-width fix by calling RenderBox::computeReplacedLogicalWidth/Height failed because those methods
call intrinsicLogicalWidth()/Height(). Becuase m_intrinsicSize is out-of-date and does not reflect
the values we just obtained from the contentRenderer, we use the default 300x150 values for object
and fail to render.
In order to both fix SVG/<object> with max-width constraints and to keep <object> rendering correctly
even when there are no max-width constraints, I was forced to update the m_intrinsicSize immediately
in order to make sure that the RenderBox methods returned the right values when computing the width/height
constrained to max/min-width/height values.
Reviewed by Dan Bernstein.
Added two new tests in svg/as-image. One test covers non-rectangular images to test for the inversion
mistake I made. The second test applies a max-width to <object> and shows that we have never gotten
this right before. An existing test in svg/as-image/ already covers basic <object> use (and tests that
the intrinsic size of 300x150 is not used when an explicit non-percentage size is specified on the SVG
itself).
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeAspectRatioInformationForRenderBox):
Add a check to update m_intrinsicSize when we know it should apply, so that the calls to check against
min/max-width fetch this correct size.
* rendering/RenderReplaced.h:
(RenderReplaced):
Make m_intrinsicSize mutable because of the mutation that occurs during the method above. It may be
that we should re-evaluate whether all of these methods should be const, but this would impact RenderBox
methods as well, so I chose to hold off going down that rabbit hole.
LayoutTests:
Reviewed by Dan Bernstein.
* svg/as-image/svg-intrinsic-size-rectangular-expected.html: Added.
* svg/as-image/svg-intrinsic-size-rectangular.html: Added.
* svg/as-image/svg-object-intrinsic-size-expected.html: Added.
* svg/as-image/svg-object-intrinsic-size.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (123182 => 123183)
--- trunk/LayoutTests/ChangeLog 2012-07-20 06:06:04 UTC (rev 123182)
+++ trunk/LayoutTests/ChangeLog 2012-07-20 06:12:54 UTC (rev 123183)
@@ -1,3 +1,15 @@
+2012-07-19 David Hyatt <[email protected]>
+
+ SVG not properly respecting max-width.
+ https://bugs.webkit.org/show_bug.cgi?id=91474
+
+ Reviewed by Dan Bernstein.
+
+ * svg/as-image/svg-intrinsic-size-rectangular-expected.html: Added.
+ * svg/as-image/svg-intrinsic-size-rectangular.html: Added.
+ * svg/as-image/svg-object-intrinsic-size-expected.html: Added.
+ * svg/as-image/svg-object-intrinsic-size.html: Added.
+
2012-07-19 Dominic Mazzoni <[email protected]>
AX: Need AccessibilityObjects for nodes without renderers in canvas subtree
Added: trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular-expected.html (0 => 123183)
--- trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular-expected.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular-expected.html 2012-07-20 06:12:54 UTC (rev 123183)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="position: absolute; top: 10px; left: 10px; width: 200px; height: 400px; outline: 1px solid blue;">
+ <div style="height:100px;background-color:black"></div>
+</div>
Added: trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular.html (0 => 123183)
--- trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-intrinsic-size-rectangular.html 2012-07-20 06:12:54 UTC (rev 123183)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="position: absolute; top: 10px; left: 10px; width: 200px; height: 400px; outline: 1px solid blue;">
+ <img style="background-color: yellow; width: auto; height: auto; max-width: 100%; max-height: 100%;" src="" width='600' height='300' xmlns='http://www.w3.org/2000/svg'><rect x='0' y='0' width='600' height='300'></rect></svg>">
+</div>
Modified: trunk/LayoutTests/svg/as-image/svg-non-integer-scaled-image-expected.txt (123182 => 123183)
--- trunk/LayoutTests/svg/as-image/svg-non-integer-scaled-image-expected.txt 2012-07-20 06:06:04 UTC (rev 123182)
+++ trunk/LayoutTests/svg/as-image/svg-non-integer-scaled-image-expected.txt 2012-07-20 06:12:54 UTC (rev 123183)
@@ -3,5 +3,5 @@
layer at (0,0) size 800x600
RenderBlock {HTML} at (0,0) size 800x600
RenderBody {BODY} at (0,0) size 800x600
- RenderImage {IMG} at (0,0) size 100x100
+ RenderImage {IMG} at (0,0) size 99x100
RenderText {#text} at (0,0) size 0x0
Added: trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size-expected.html (0 => 123183)
--- trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size-expected.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size-expected.html 2012-07-20 06:12:54 UTC (rev 123183)
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<div style="width:200px;height:200px;outline:1px solid blue;margin:5px;float:left"><img style="background-color: yellow; width: auto; height: auto; max-width: 100%; max-height: 100%;" src=""
+</div>
+
+<div style="width:200px;height:200px;outline:1px solid blue;margin:5px;float:left">
+ <img id="target" style="background-color: yellow; width: auto; height: auto; max-width: 100%; max-height: 100%;" src=""
+</div>
Added: trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size.html (0 => 123183)
--- trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-object-intrinsic-size.html 2012-07-20 06:12:54 UTC (rev 123183)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<object style="margin:5px; outline: 1px solid blue; max-width:200px" data="" width='300' height='300' xmlns='http://www.w3.org/2000/svg'><rect x='0' y='0' width='300' height='300'></rect></svg>">
+</object><object style="margin:5px; outline: 1px solid blue; max-width:200px" data=""
+</object>
Modified: trunk/Source/WebCore/ChangeLog (123182 => 123183)
--- trunk/Source/WebCore/ChangeLog 2012-07-20 06:06:04 UTC (rev 123182)
+++ trunk/Source/WebCore/ChangeLog 2012-07-20 06:12:54 UTC (rev 123183)
@@ -1,3 +1,40 @@
+2012-07-19 David Hyatt <[email protected]>
+
+ SVG not properly respecting max-width.
+ https://bugs.webkit.org/show_bug.cgi?id=91474
+
+ My previous checkin for 91474 accidentally inverted width and height in the division case for
+ the computation of height. When I fixed this inversion, I discovered that <object> elements
+ are in fact also broken with max-width handling, and that furthermore, trying to apply the same
+ max-width fix by calling RenderBox::computeReplacedLogicalWidth/Height failed because those methods
+ call intrinsicLogicalWidth()/Height(). Becuase m_intrinsicSize is out-of-date and does not reflect
+ the values we just obtained from the contentRenderer, we use the default 300x150 values for object
+ and fail to render.
+
+ In order to both fix SVG/<object> with max-width constraints and to keep <object> rendering correctly
+ even when there are no max-width constraints, I was forced to update the m_intrinsicSize immediately
+ in order to make sure that the RenderBox methods returned the right values when computing the width/height
+ constrained to max/min-width/height values.
+
+ Reviewed by Dan Bernstein.
+
+ Added two new tests in svg/as-image. One test covers non-rectangular images to test for the inversion
+ mistake I made. The second test applies a max-width to <object> and shows that we have never gotten
+ this right before. An existing test in svg/as-image/ already covers basic <object> use (and tests that
+ the intrinsic size of 300x150 is not used when an explicit non-percentage size is specified on the SVG
+ itself).
+
+ * rendering/RenderReplaced.cpp:
+ (WebCore::RenderReplaced::computeAspectRatioInformationForRenderBox):
+ Add a check to update m_intrinsicSize when we know it should apply, so that the calls to check against
+ min/max-width fetch this correct size.
+
+ * rendering/RenderReplaced.h:
+ (RenderReplaced):
+ Make m_intrinsicSize mutable because of the mutation that occurs during the method above. It may be
+ that we should re-evaluate whether all of these methods should be const, but this would impact RenderBox
+ methods as well, so I chose to hold off going down that rabbit hole.
+
2012-07-19 Dominic Mazzoni <[email protected]>
AX: Need AccessibilityObjects for nodes without renderers in canvas subtree
Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (123182 => 123183)
--- trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-07-20 06:06:04 UTC (rev 123182)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-07-20 06:12:54 UTC (rev 123183)
@@ -286,6 +286,12 @@
if (rendererHasAspectRatio(this) && isPercentageIntrinsicSize)
intrinsicRatio = 1;
+
+ // Update our intrinsic size to match what the content renderer has computed, so that when we
+ // constrain the size below, the correct intrinsic size will be obtained for comparison against
+ // min and max widths.
+ if (intrinsicRatio && !isPercentageIntrinsicSize && !intrinsicSize.isEmpty())
+ m_intrinsicSize = flooredIntSize(intrinsicSize); // FIXME: This introduces precision errors. We should convert m_intrinsicSize to be a float.
} else {
computeIntrinsicRatioInformation(intrinsicSize, intrinsicRatio, isPercentageIntrinsicSize);
if (intrinsicRatio)
@@ -299,11 +305,11 @@
// FIXME: In the long term, it might be better to just return this code more to the way it used to be before this
// function was added, since all it has done is make the code more unclear.
constrainedSize = intrinsicSize;
- if (intrinsicRatio && !intrinsicSize.isEmpty() && style()->logicalWidth().isAuto() && style()->logicalHeight().isAuto()) {
+ if (intrinsicRatio && !isPercentageIntrinsicSize && !intrinsicSize.isEmpty() && style()->logicalWidth().isAuto() && style()->logicalHeight().isAuto()) {
// We can't multiply or divide by 'intrinsicRatio' here, it breaks tests, like fast/images/zoomed-img-size.html, which
// can only be fixed once subpixel precision is available for things like intrinsicWidth/Height - which include zoom!
constrainedSize.setWidth(RenderBox::computeReplacedLogicalHeight() * intrinsicSize.width() / intrinsicSize.height());
- constrainedSize.setHeight(RenderBox::computeReplacedLogicalWidth() * intrinsicSize.width() / intrinsicSize.height());
+ constrainedSize.setHeight(RenderBox::computeReplacedLogicalWidth() * intrinsicSize.height() / intrinsicSize.width());
}
}
Modified: trunk/Source/WebCore/rendering/RenderReplaced.h (123182 => 123183)
--- trunk/Source/WebCore/rendering/RenderReplaced.h 2012-07-20 06:06:04 UTC (rev 123182)
+++ trunk/Source/WebCore/rendering/RenderReplaced.h 2012-07-20 06:12:54 UTC (rev 123183)
@@ -79,7 +79,7 @@
virtual LayoutRect selectionRectForRepaint(RenderBoxModelObject* repaintContainer, bool clipToVisibleContent = true);
void computeAspectRatioInformationForRenderBox(RenderBox*, FloatSize& constrainedSize, double& intrinsicRatio, bool& isPercentageIntrinsicSize) const;
- IntSize m_intrinsicSize;
+ mutable IntSize m_intrinsicSize;
};
}
