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 19393c7af2804ea660390c92f570381cbd255a30..3968f1a37e8277a7374ba12df6fedaf43b7b53f5 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 fb8d937fd85f306b78f3ca460d6a8222255993b4..bb56f175aea44e33851cb2363ab1f61be7749bd6 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 a19141ae71c400dae75f550b9fc5d7ceb07e4979..02c8273c2729dd078a774243aa9781c5ce0d2e8a 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 c80afc9810e1b77f313b1739a2e71017c9f1bb8c..83dc53438c2232663d0e6adf614513f2c56a71a7 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 96af0e81e15b0b300d1ea038f9bc1d44e9bf996b..091303cfffcde0f872d49f3983ec38f47c3389df 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 4357fe1712a5c892460e0a1c5c304ab3ee5f5aa2..9ab353c66fe7feae0a5e679868f561a3653b1ac0 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 6080d7aa935187615fe301c00b6c09f986f00c9a..9ff0a3cc72e83c85797b447012aecedc21883784 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 e2ad696aeffc0de9c82902ac172c94af8562dff6..34fe7a6d4de95b7cf15f9451e822f8065e7087b6 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;