Fix handling of top margin on float with shape-outside
authorbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Sep 2013 18:18:39 +0000 (18:18 +0000)
committerbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Sep 2013 18:18:39 +0000 (18:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121614

Reviewed by Alexandru Chiculita.

Source/WebCore:

When a float has shape outside, the top margin should be treated as if
there is no shape there, so inline content should be allowed to flow
into that space. This patch fixes two issues:

1) If the top margin is the same as the line height, a line should be
able to fit into the margin. Before this patch, that line was being
treated as if it intersected with the shape.

2) The shape should be positioned (x, y) relative to the box sizing
box of the float. While the x coordinate was being treated properly,
the y coordinate was relative to the top of the margin box. This patch
fixes this behavior.

This patch also includes a simple test for right and left margins, as
I wrote that test and then discovered the problems listed above.

This patch also removes an unused override of the
lineOverlapsShapeBounds method.

Tests: csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html
       csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html

* rendering/FloatingObjects.cpp:
(WebCore::FloatingObjects::logicalLeftOffset):
(WebCore::FloatingObjects::logicalRightOffset):
* rendering/LineWidth.cpp:
(WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded):
* rendering/shapes/ShapeInfo.h:
* rendering/shapes/ShapeInsideInfo.h:
* rendering/shapes/ShapeOutsideInfo.cpp:
(WebCore::ShapeOutsideInfo::computeSegmentsForContainingBlockLine):
* rendering/shapes/ShapeOutsideInfo.h:

LayoutTests:

* csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000-expected.html: Added.
* csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html: Added.
    Test for a positive left/right margin.

* csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001-expected.html: Added.
* csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html: Added.
    Test for a positive top margin.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000-expected.html [new file with mode: 0644]
LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html [new file with mode: 0644]
LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001-expected.html [new file with mode: 0644]
LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/FloatingObjects.cpp
Source/WebCore/rendering/LineWidth.cpp
Source/WebCore/rendering/shapes/ShapeInfo.h
Source/WebCore/rendering/shapes/ShapeInsideInfo.h
Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp
Source/WebCore/rendering/shapes/ShapeOutsideInfo.h

index 19393c7..3968f1a 100644 (file)
@@ -1,3 +1,18 @@
+2013-09-19  Bem Jones-Bey  <bjonesbe@adobe.com>
+
+        Fix handling of top margin on float with shape-outside
+        https://bugs.webkit.org/show_bug.cgi?id=121614
+
+        Reviewed by Alexandru Chiculita.
+
+        * csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000-expected.html: Added.
+        * csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html: Added.
+            Test for a positive left/right margin.
+
+        * csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001-expected.html: Added.
+        * csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html: Added.
+            Test for a positive top margin.
+
 2013-09-19  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add XHR tests checking readyState transition when abort() is invoked in various states
diff --git a/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000-expected.html b/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000-expected.html
new file mode 100644 (file)
index 0000000..c37fc73
--- /dev/null
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<title>CSS Reference</title>
+<link rel="author" title="Adobe" href="http://html.adobe.com/">
+<link rel="author" title="Bem Jones-Bey" href="mailto:bjonesbe@adobe.com">
+<link rel="help" href="http://dev.w3.org/csswg/css-shapes-1/#shape-margin-property">
+<meta name="flags" content="ahem">
+<head>
+<style>
+.container {
+    font: 20px/1 Ahem, sans-serif;
+    line-height: 20px;
+    width: 100px;
+    height: 20px;
+    border: 1px solid black;
+    color: green;
+    display: inline-block;
+}
+
+#float-left {
+    float: left;
+    width: 20px;
+    height: 20px;
+}
+
+#float-right {
+    float: right;
+    width: 20px;
+    height: 20px;
+}
+</style>
+
+<body>
+    <p>This should display two green bars, with white squares on opposite sides.</p>
+    <div class="container">
+        <div id="float-left">
+        </div>
+        XXXX
+    </div>
+    <div class="container">
+        <div id="float-right">
+        </div>
+        XXXX
+    </div>
+</body>
diff --git a/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html b/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html
new file mode 100644 (file)
index 0000000..c61bde1
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<title>CSS Test: shape-outside on floats with a positive margin</title>
+<link rel="author" title="Adobe" href="http://html.adobe.com/">
+<link rel="author" title="Bem Jones-Bey" href="mailto:bjonesbe@adobe.com">
+<link rel="help" href="http://dev.w3.org/csswg/css-shapes-1/#shape-margin-property">
+<link rel="match" href="shape-outside-floats-margin-000-ref.html">
+<meta name="flags" content="ahem">
+<head>
+<style>
+.container {
+    font: 20px/1 Ahem, sans-serif;
+    line-height: 20px;
+    width: 100px;
+    height: 20px;
+    border: 1px solid black;
+    color: green;
+    display: inline-block;
+}
+
+#float-left {
+    float: left;
+    width: 100px;
+    height: 20px;
+    -webkit-shape-outside: rectangle(0, 0, 0, 100%);
+    margin-left: 20px;
+}
+
+#float-right {
+    float: right;
+    width: 100px;
+    height: 20px;
+    -webkit-shape-outside: rectangle(0, 0, 0, 100%);
+    margin-right: 20px;
+}
+</style>
+
+<body>
+    <p>This should display two green bars, with white squares on opposite sides.</p>
+    <div class="container">
+        <div id="float-left">
+        </div>
+        XXXX
+    </div>
+    <div class="container">
+        <div id="float-right">
+        </div>
+        XXXX
+    </div>
+</body>
diff --git a/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001-expected.html b/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001-expected.html
new file mode 100644 (file)
index 0000000..25c9125
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<title>CSS Reference</title>
+<link rel="author" title="Adobe" href="http://html.adobe.com/">
+<link rel="author" title="Bem Jones-Bey" href="mailto:bjonesbe@adobe.com">
+<link rel="help" href="http://dev.w3.org/csswg/css-shapes-1/#shape-margin-property">
+<meta name="flags" content="ahem">
+<head>
+<style>
+.container {
+    font: 20px/1 Ahem, sans-serif;
+    line-height: 20px;
+    width: 100px;
+    height: 40px;
+    border: 1px solid black;
+    color: green;
+    display: inline-block;
+    background-color: red;
+}
+
+#float-left {
+    float: left;
+    width: 100px;
+    height: 20px;
+    background-color: green;
+}
+
+#float-right {
+    float: right;
+    width: 100px;
+    height: 20px;
+    background-color: green;
+}
+</style>
+
+<body>
+    <p>This should display two green rectangles. You should not see any red.</p>
+    <div class="container">
+        XXXXX
+        <div id="float-left">
+        </div>
+    </div>
+    <div class="container">
+        XXXXX
+        <div id="float-right">
+        </div>
+    </div>
+</body>
diff --git a/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html b/LayoutTests/csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html
new file mode 100644 (file)
index 0000000..ab57820
--- /dev/null
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<title>CSS Test: shape-outside on floats with a positive top margin</title>
+<link rel="author" title="Adobe" href="http://html.adobe.com/">
+<link rel="author" title="Bem Jones-Bey" href="mailto:bjonesbe@adobe.com">
+<link rel="help" href="http://dev.w3.org/csswg/css-shapes-1/#shape-margin-property">
+<link rel="match" href="shape-outside-floats-margin-001-ref.html">
+<meta name="flags" content="ahem">
+<head>
+<style>
+.container {
+    font: 20px/1 Ahem, sans-serif;
+    line-height: 20px;
+    width: 100px;
+    height: 40px;
+    border: 1px solid black;
+    color: green;
+    display: inline-block;
+    background-color: red;
+}
+
+#float-left {
+    float: left;
+    width: 100px;
+    height: 20px;
+    -webkit-shape-outside: rectangle(0, 0, 50%, 100%);
+    margin-top: 20px;
+    background-color: green;
+}
+
+#float-right {
+    float: right;
+    width: 100px;
+    height: 20px;
+    -webkit-shape-outside: rectangle(0, 0, 50%, 100%);
+    margin-top: 20px;
+    background-color: green;
+}
+</style>
+
+<body>
+    <p>This should display two green rectangles. You should not see any red.</p>
+    <div class="container">
+        <div id="float-left">
+        </div>
+        XXXXX
+    </div>
+    <div class="container">
+        <div id="float-right">
+        </div>
+        XXXXX
+    </div>
+</body>
index fb8d937..bb56f17 100644 (file)
@@ -1,3 +1,43 @@
+2013-09-19  Bem Jones-Bey  <bjonesbe@adobe.com>
+
+        Fix handling of top margin on float with shape-outside
+        https://bugs.webkit.org/show_bug.cgi?id=121614
+
+        Reviewed by Alexandru Chiculita.
+
+        When a float has shape outside, the top margin should be treated as if
+        there is no shape there, so inline content should be allowed to flow
+        into that space. This patch fixes two issues:
+        
+        1) If the top margin is the same as the line height, a line should be
+        able to fit into the margin. Before this patch, that line was being
+        treated as if it intersected with the shape.
+        
+        2) The shape should be positioned (x, y) relative to the box sizing
+        box of the float. While the x coordinate was being treated properly,
+        the y coordinate was relative to the top of the margin box. This patch
+        fixes this behavior.
+        
+        This patch also includes a simple test for right and left margins, as
+        I wrote that test and then discovered the problems listed above.
+        
+        This patch also removes an unused override of the
+        lineOverlapsShapeBounds method.
+
+        Tests: csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-000.html
+               csswg/submitted/shapes/shape-outside/shape-outside-floats-margin-001.html
+
+        * rendering/FloatingObjects.cpp:
+        (WebCore::FloatingObjects::logicalLeftOffset): 
+        (WebCore::FloatingObjects::logicalRightOffset):
+        * rendering/LineWidth.cpp:
+        (WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded):
+        * rendering/shapes/ShapeInfo.h:
+        * rendering/shapes/ShapeInsideInfo.h:
+        * rendering/shapes/ShapeOutsideInfo.cpp:
+        (WebCore::ShapeOutsideInfo::computeSegmentsForContainingBlockLine):
+        * rendering/shapes/ShapeOutsideInfo.h:
+
 2013-09-19  Antti Koivisto  <antti@apple.com>
 
         Add RenderElement
index a19141a..02c8273 100644 (file)
@@ -288,7 +288,7 @@ LayoutUnit FloatingObjects::logicalLeftOffset(LayoutUnit fixedOffset, LayoutUnit
     const FloatingObject* outermostFloat = adapter.outermostFloat();
     if (offsetMode == ShapeOutsideFloatShapeOffset && outermostFloat) {
         if (ShapeOutsideInfo* shapeOutside = outermostFloat->renderer()->shapeOutsideInfo()) {
-            shapeOutside->computeSegmentsForContainingBlockLine(logicalTop, outermostFloat->logicalTop(m_horizontalWritingMode), logicalHeight);
+            shapeOutside->computeSegmentsForContainingBlockLine(m_renderer, outermostFloat, logicalTop, logicalHeight);
             offset += shapeOutside->rightSegmentMarginBoxDelta();
         }
     }
@@ -314,7 +314,7 @@ LayoutUnit FloatingObjects::logicalRightOffset(LayoutUnit fixedOffset, LayoutUni
     const FloatingObject* outermostFloat = adapter.outermostFloat();
     if (offsetMode == ShapeOutsideFloatShapeOffset && outermostFloat) {
         if (ShapeOutsideInfo* shapeOutside = outermostFloat->renderer()->shapeOutsideInfo()) {
-            shapeOutside->computeSegmentsForContainingBlockLine(logicalTop, outermostFloat->logicalTop(m_horizontalWritingMode), logicalHeight);
+            shapeOutside->computeSegmentsForContainingBlockLine(m_renderer, outermostFloat, logicalTop, logicalHeight);
             offset += shapeOutside->leftSegmentMarginBoxDelta();
         }
     }
index c80afc9..83dc534 100644 (file)
@@ -107,14 +107,14 @@ void LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject* newFloat
         if (previousFloat != newFloat && previousFloat->type() == newFloat->type()) {
             previousShapeOutsideInfo = previousFloat->renderer()->shapeOutsideInfo();
             if (previousShapeOutsideInfo)
-                previousShapeOutsideInfo->computeSegmentsForContainingBlockLine(m_block.logicalHeight(), previousFloat->logicalTop(m_block.isHorizontalWritingMode()), logicalHeightForLine(&m_block, m_isFirstLine));
+                previousShapeOutsideInfo->computeSegmentsForContainingBlockLine(&m_block, previousFloat, m_block.logicalHeight(), logicalHeightForLine(&m_block, m_isFirstLine));
             break;
         }
     }
 
     ShapeOutsideInfo* shapeOutsideInfo = newFloat->renderer()->shapeOutsideInfo();
     if (shapeOutsideInfo)
-        shapeOutsideInfo->computeSegmentsForContainingBlockLine(m_block.logicalHeight(), newFloat->logicalTop(m_block.isHorizontalWritingMode()), logicalHeightForLine(&m_block, m_isFirstLine));
+        shapeOutsideInfo->computeSegmentsForContainingBlockLine(&m_block, newFloat, m_block.logicalHeight(), logicalHeightForLine(&m_block, m_isFirstLine));
 #endif
 
     if (newFloat->type() == FloatingObject::FloatLeft) {
index 96af0e8..091303c 100644 (file)
@@ -100,8 +100,7 @@ public:
 
     LayoutUnit shapeContainingBlockLogicalHeight() const { return (m_renderer->style()->boxSizing() == CONTENT_BOX) ? (m_shapeLogicalSize.height() + m_renderer->borderAndPaddingLogicalHeight()) : m_shapeLogicalSize.height(); }
 
-    bool lineOverlapsShapeBounds() const { return logicalLineTop() < shapeLogicalBottom() && shapeLogicalTop() <= logicalLineBottom(); }
-    bool lineOverlapsShapeBounds(LayoutUnit lineHeight) const { return logicalLineTop() < shapeLogicalBottom() && shapeLogicalTop() <= logicalLineBottom(lineHeight); }
+    virtual bool lineOverlapsShapeBounds() const = 0;
 
     void dirtyShapeSize() { m_shape.clear(); }
     bool shapeSizeDirty() { return !m_shape.get(); }
index 4357fe1..9ab353c 100644 (file)
@@ -109,6 +109,12 @@ public:
     void setNeedsLayout(bool value) { m_needsLayout = value; }
     bool needsLayout() { return m_needsLayout; }
 
+    virtual bool lineOverlapsShapeBounds() const OVERRIDE
+    {
+        // The <= test is to handle the case of a zero height line or a zero height shape.
+        return logicalLineTop() < shapeLogicalBottom() && shapeLogicalTop() <= logicalLineBottom();
+    }
+
 protected:
     virtual LayoutRect computedShapeLogicalBoundingBox() const OVERRIDE { return computedShape()->shapePaddingLogicalBoundingBox(); }
     virtual ShapeValue* shapeValue() const OVERRIDE;
index 6080d7a..9ff0a3c 100644 (file)
@@ -33,6 +33,8 @@
 
 #include "ShapeOutsideInfo.h"
 
+#include "FloatingObjects.h"
+#include "RenderBlock.h"
 #include "RenderBox.h"
 
 namespace WebCore {
@@ -52,9 +54,10 @@ bool ShapeOutsideInfo::isEnabledFor(const RenderBox* box)
     }
 }
 
-bool ShapeOutsideInfo::computeSegmentsForContainingBlockLine(LayoutUnit lineTop, LayoutUnit floatTop, LayoutUnit lineHeight)
+bool ShapeOutsideInfo::computeSegmentsForContainingBlockLine(const RenderBlock* containingBlock, const FloatingObject* floatingObject, LayoutUnit lineTop, LayoutUnit lineHeight)
 {
-    LayoutUnit lineTopInShapeCoordinates = lineTop - floatTop + logicalTopOffset();
+    LayoutUnit shapeTop = floatingObject->logicalTop(containingBlock->isHorizontalWritingMode()) + std::max(LayoutUnit(), containingBlock->marginBeforeForChild(m_renderer));
+    LayoutUnit lineTopInShapeCoordinates = lineTop - shapeTop + logicalTopOffset();
     return updateSegmentsForLine(lineTopInShapeCoordinates, lineHeight);
 }
 
index e2ad696..34fe7a6 100644 (file)
 
 namespace WebCore {
 
+class RenderBlock;
 class RenderBox;
+class FloatingObject;
 
 class ShapeOutsideInfo FINAL : public ShapeInfo<RenderBox>, public MappedInfo<RenderBox, ShapeOutsideInfo> {
 public:
     LayoutUnit leftSegmentMarginBoxDelta() const { return m_leftSegmentMarginBoxDelta; }
     LayoutUnit rightSegmentMarginBoxDelta() const { return m_rightSegmentMarginBoxDelta; }
 
-    bool computeSegmentsForContainingBlockLine(LayoutUnit lineTop, LayoutUnit floatTop, LayoutUnit lineHeight);
+    bool computeSegmentsForContainingBlockLine(const RenderBlock*, const FloatingObject*, LayoutUnit lineTop, LayoutUnit lineHeight);
     virtual bool updateSegmentsForLine(LayoutUnit lineTop, LayoutUnit lineHeight) OVERRIDE;
 
     static PassOwnPtr<ShapeOutsideInfo> createInfo(const RenderBox* renderer) { return adoptPtr(new ShapeOutsideInfo(renderer)); }
     static bool isEnabledFor(const RenderBox*);
 
+    virtual bool lineOverlapsShapeBounds() const OVERRIDE
+    {
+        return (logicalLineTop() < shapeLogicalBottom() && shapeLogicalTop() < logicalLineBottom())
+            || logicalLineTop() == shapeLogicalTop(); // case of zero height line or zero height shape
+    }
+
 protected:
     virtual LayoutRect computedShapeLogicalBoundingBox() const OVERRIDE { return computedShape()->shapeMarginLogicalBoundingBox(); }
     virtual ShapeValue* shapeValue() const OVERRIDE;