Changeset 101755 in webkit

Timestamp:

Dec 2, 2011 12:26:05 AM (12 years ago)

Author:

commit-queue@webkit.org

Message:

Range sliders and spin buttons don't work with multi-columns.
​https://bugs.webkit.org/show_bug.cgi?id=70898

Patch by Yosifumi Inoue <​yosin@chromium.org> on 2011-12-02
Reviewed by Dan Bernstein.

Source/WebCore:

This patch makes RenderBlock::hitTestColumns and
RenderBoxModelObject::mapAbsoluteToLocal to handle point
in multi-column same logic.

In multi-column, coordinate of box model rendering object is different
from absolute coordinate.. Columns in box model rendering object spans
vertically rather than horizontally.

When absolute point is represented in (column[i]+dx, column[0]+dy),
it is (column[0]+dx, column[0] + columnHeight + dy) in box model
rendering object coordinate.

Tests: fast/events/document-elementFromPoint.html

fast/events/offsetX-offsetY.html
fast/forms/number/spin-in-multi-column.html
fast/forms/range/slider-in-multi-column.html
fast/forms/select/listbox-in-multi-column.html

• rendering/RenderBlock.cpp:

(WebCore::ColumnRectIterator::ColumnRectIterator): Added
(WebCore::ColumnRectIterator::advance): Added
(WebCore::ColumnRectIterator::columnRect): Added
(WebCore::ColumnRectIterator::hasMore): Added
(WebCore::ColumnRectIterator::adjust): Added
(WebCore::ColumnRectIterator::update): Added
(WebCore::RenderBlock::hitTestColumns): Use ColumnRectIterator.
(WebCore::RenderBlock::adjustForColumnRect): Added

• rendering/RenderBlock.h: Add adjustForColumnRect.
• rendering/RenderBox.cpp:

(WebCore::RenderBox::mapAbsoluteToLocalPoint): Call RenderBoxModelObject::mapAbsoluteToLocalPoint.

• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::mapAbsoluteToLocalPoint): Move from RenderInline::mapAbsoluteToLocalPoint and call RenderBlock::adjustForColumnRect.

• rendering/RenderBoxModelObject.h: add mapAbsoluteToLocalPoint.
• rendering/RenderInline.cpp: Move mapAbsoluteToLocalPoint to RenderBoxModelObject.
• rendering/RenderInline.h: remove mapAbsoluteToLocalPoint.

LayoutTests:

• fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
• fast/events/offsetX-offsetY.html: Change offset for "in-columns" test.
• fast/forms/listbox/listbox-in-multi-column-expected.txt: Added.
• fast/forms/listbox/listbox-in-multi-column.html: Added.
• fast/forms/number/spin-in-multi-column-expected.txt: Added.
• fast/forms/number/spin-in-multi-column.html: Added.
• fast/forms/range/slider-in-multi-column-expected.txt: Added.
• fast/forms/range/slider-in-multi-column.html: Added.
• platform/chromium-win/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
• platform/efl/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
• platform/gtk/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/offsetX-offsetY-expected.txt (modified) (1 diff)
• LayoutTests/fast/events/offsetX-offsetY.html (modified) (1 diff)
• LayoutTests/fast/forms/number (added)
• LayoutTests/fast/forms/number/spin-in-multi-column-expected.txt (added)
• LayoutTests/fast/forms/number/spin-in-multi-column.html (added)
• LayoutTests/fast/forms/range/slider-in-multi-column-expected.txt (added)
• LayoutTests/fast/forms/range/slider-in-multi-column.html (added)
• LayoutTests/fast/forms/select (added)
• LayoutTests/fast/forms/select/listbox-in-multi-column-expected.txt (added)
• LayoutTests/fast/forms/select/listbox-in-multi-column.html (added)
• LayoutTests/platform/chromium-win/fast/events/offsetX-offsetY-expected.txt (modified) (1 diff)
• LayoutTests/platform/efl/fast/events/offsetX-offsetY-expected.txt (modified) (1 diff)
• LayoutTests/platform/gtk/fast/events/offsetX-offsetY-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBoxModelObject.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderBoxModelObject.h (modified) (1 diff)
• Source/WebCore/rendering/RenderInline.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderInline.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-12-02  Yosifumi Inoue  <yosin@chromium.org>
+
+        Range sliders and spin buttons don't work with multi-columns.
+        https://bugs.webkit.org/show_bug.cgi?id=70898
+
+        Reviewed by Dan Bernstein.
+
+        * fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
+        * fast/events/offsetX-offsetY.html: Change offset for "in-columns" test.
+        * fast/forms/listbox/listbox-in-multi-column-expected.txt: Added.
+        * fast/forms/listbox/listbox-in-multi-column.html: Added.
+        * fast/forms/number/spin-in-multi-column-expected.txt: Added.
+        * fast/forms/number/spin-in-multi-column.html: Added.
+        * fast/forms/range/slider-in-multi-column-expected.txt: Added.
+        * fast/forms/range/slider-in-multi-column.html: Added.
+        * platform/chromium-win/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
+        * platform/efl/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
+        * platform/gtk/fast/events/offsetX-offsetY-expected.txt: Change offset for "in-columns" test.
+
 2011-12-02  Pavel Feldman  <pfeldman@google.com>
 

trunk/LayoutTests/fast/events/offsetX-offsetY-expected.txt

@@ -19 +19 @@
 PASS: event at (410, 30) hit fixed-box at offset (10, 10)
 PASS: event at (36, 272) hit with-bordertopextra at offset (4, 4)
-PASS: event at (639, 207) hit in-columns at offset (173, -189)
+PASS: event at (639, 207) hit in-columns at offset (35, 5)
 PASS: event at (563, 410) hit inside-overflow at offset (7, 6)
 PASS: event at (112, 369) hit transformed at offset (11, 16)

trunk/LayoutTests/fast/events/offsetX-offsetY.html

@@ -38 +38 @@
       dispatchEvent(410, 30, 'fixed-box', 10, 10);
       dispatchEvent(36, 272, 'with-bordertopextra', 4, 4);
-      dispatchEvent(639, 207, 'in-columns', 173, -189);
+      dispatchEvent(639, 207, 'in-columns', 35, 5);
       dispatchEvent(563, 410, 'inside-overflow', 7, 6);
       dispatchEvent(112, 369, 'transformed', 11, 16);

trunk/LayoutTests/platform/chromium-win/fast/events/offsetX-offsetY-expected.txt

@@ -19 +19 @@
 PASS: event at (410, 30) hit fixed-box at offset (10, 10)
 FAIL: event at (36, 272) expected to hit with-bordertopextra at (4, 4) but hit filler at (56, 372)
-FAIL: event at (639, 207) expected to hit in-columns at (173, -189) but hit in-columns at (173, -205)
+PASS: event at (639, 207) hit in-columns at offset (35, 5)
 FAIL: event at (563, 410) expected to hit inside-overflow at (7, 6) but hit overflow-contents at (178, 112)
 FAIL: event at (112, 369) expected to hit transformed at (11, 16) but hit transformed at (7, 2)

trunk/LayoutTests/platform/efl/fast/events/offsetX-offsetY-expected.txt

@@ -19 +19 @@
 PASS: event at (410, 30) hit fixed-box at offset (10, 10)
 FAIL: event at (36, 272) expected to hit with-bordertopextra at (4, 4) but hit with-bordertopextra at (4, 0)
-FAIL: event at (639, 207) expected to hit in-columns at (173, -189) but hit in-columns at (173, -210)
+PASS: event at (639, 207) hit in-columns at offset (35, 5)
 FAIL: event at (563, 410) expected to hit inside-overflow at (7, 6) but hit inside-overflow at (7, 1)
 FAIL: event at (112, 369) expected to hit transformed at (11, 16) but hit transformed at (9, 9)

trunk/LayoutTests/platform/gtk/fast/events/offsetX-offsetY-expected.txt

@@ -19 +19 @@
 PASS: event at (410, 30) hit fixed-box at offset (10, 10)
 PASS: event at (36, 272) hit with-bordertopextra at offset (4, 4)
-FAIL: event at (639, 207) expected to hit in-columns at (173, -189) but hit in-columns at (173, -199)
+PASS: event at (639, 207) hit in-columns at offset (35, 5)
 PASS: event at (563, 410) hit inside-overflow at offset (7, 6)
 PASS: event at (112, 369) hit transformed at offset (11, 16)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-12-02  Yosifumi Inoue  <yosin@chromium.org>
+
+        Range sliders and spin buttons don't work with multi-columns.
+        https://bugs.webkit.org/show_bug.cgi?id=70898
+
+        Reviewed by Dan Bernstein.
+
+        This patch makes RenderBlock::hitTestColumns and
+        RenderBoxModelObject::mapAbsoluteToLocal to handle point
+        in multi-column same logic.
+
+        In multi-column, coordinate of box model rendering object is different
+        from absolute coordinate.. Columns in box model rendering object spans
+        vertically rather than horizontally.
+
+        When absolute point is represented in (column[i]+dx, column[0]+dy),
+        it is (column[0]+dx, column[0] + columnHeight + dy) in box model
+        rendering object coordinate.
+
+        Tests: fast/events/document-elementFromPoint.html
+               fast/events/offsetX-offsetY.html
+               fast/forms/number/spin-in-multi-column.html
+               fast/forms/range/slider-in-multi-column.html
+               fast/forms/select/listbox-in-multi-column.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::ColumnRectIterator::ColumnRectIterator): Added
+        (WebCore::ColumnRectIterator::advance): Added
+        (WebCore::ColumnRectIterator::columnRect): Added
+        (WebCore::ColumnRectIterator::hasMore): Added
+        (WebCore::ColumnRectIterator::adjust): Added
+        (WebCore::ColumnRectIterator::update): Added
+        (WebCore::RenderBlock::hitTestColumns): Use ColumnRectIterator.
+        (WebCore::RenderBlock::adjustForColumnRect): Added
+        * rendering/RenderBlock.h: Add adjustForColumnRect.
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::mapAbsoluteToLocalPoint): Call RenderBoxModelObject::mapAbsoluteToLocalPoint.
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::mapAbsoluteToLocalPoint): Move from RenderInline::mapAbsoluteToLocalPoint and call RenderBlock::adjustForColumnRect.
+        * rendering/RenderBoxModelObject.h: add mapAbsoluteToLocalPoint.
+        * rendering/RenderInline.cpp: Move mapAbsoluteToLocalPoint to RenderBoxModelObject.
+        * rendering/RenderInline.h: remove mapAbsoluteToLocalPoint.
+
 2011-12-02  Pavel Feldman  <pfeldman@google.com>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -4238 +4238 @@
 }
 
+class ColumnRectIterator {
+    WTF_MAKE_NONCOPYABLE(ColumnRectIterator);
+public:
+    ColumnRectIterator(const RenderBlock& block)
+        : m_block(block)
+        , m_colInfo(block.columnInfo())
+        , m_direction(m_block.style()->isFlippedBlocksWritingMode() ? 1 : -1)
+        , m_isHorizontal(block.isHorizontalWritingMode())
+        , m_logicalLeft(block.logicalLeftOffsetForContent())
+    {
+        int colCount = m_colInfo->columnCount();
+        m_colIndex = colCount - 1;
+        m_currLogicalTopOffset = colCount * m_colInfo->columnHeight() * m_direction;
+        update();
+    }
+
+    void advance()
+    {
+        ASSERT(hasMore());
+        m_colIndex--;
+        update();
+    }
+
+    LayoutRect columnRect() const { return m_colRect; }
+    bool hasMore() const { return m_colIndex >= 0; }
+
+    void adjust(LayoutSize& offset) const
+    {
+        LayoutUnit currLogicalLeftOffset = (m_isHorizontal ? m_colRect.x() : m_colRect.y()) - m_logicalLeft;
+        offset += m_isHorizontal ? LayoutSize(currLogicalLeftOffset, m_currLogicalTopOffset) : LayoutSize(m_currLogicalTopOffset, currLogicalLeftOffset);
+        if (m_colInfo->progressionAxis() == ColumnInfo::BlockAxis) {
+            if (m_isHorizontal)
+                offset.expand(0, m_colRect.y() - m_block.borderTop() - m_block.paddingTop());
+            else
+                offset.expand(m_colRect.x() - m_block.borderLeft() - m_block.paddingLeft(), 0);
+        }
+    }
+
+private:
+    void update()
+    {
+        if (m_colIndex < 0)
+            return;
+
+        m_colRect = m_block.columnRectAt(const_cast<ColumnInfo*>(m_colInfo), m_colIndex);
+        m_block.flipForWritingMode(m_colRect);
+        m_currLogicalTopOffset -= (m_isHorizontal ? m_colRect.height() : m_colRect.width()) * m_direction;
+    }
+
+    const RenderBlock& m_block;
+    const ColumnInfo* const m_colInfo;
+    const int m_direction;
+    const bool m_isHorizontal;
+    const LayoutUnit m_logicalLeft;
+    int m_colIndex;
+    LayoutUnit m_currLogicalTopOffset;
+    LayoutRect m_colRect;
+};
+
 bool RenderBlock::hitTestColumns(const HitTestRequest& request, HitTestResult& result, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction hitTestAction)
 {
     // We need to do multiple passes, breaking up our hit testing into strips.
-    ColumnInfo* colInfo = columnInfo();
-    int colCount = columnCount(colInfo);
-    if (!colCount)
+    if (!hasColumns())
         return false;
-    LayoutUnit logicalLeft = logicalLeftOffsetForContent();
-    LayoutUnit currLogicalTopOffset = !style()->isFlippedBlocksWritingMode() ? -colCount * colInfo->columnHeight() : colCount * colInfo->columnHeight();
-    bool isHorizontal = isHorizontalWritingMode();
-
-    for (int i = colCount - 1; i >= 0; i--) {
-        LayoutRect colRect = columnRectAt(colInfo, i);
-        flipForWritingMode(colRect);
-        LayoutUnit currLogicalLeftOffset = (isHorizontal ? colRect.x() : colRect.y()) - logicalLeft;
-        LayoutUnit blockDelta =  (isHorizontal ? colRect.height() : colRect.width());
-        if (style()->isFlippedBlocksWritingMode())
-            currLogicalTopOffset -= blockDelta;
-        else
-            currLogicalTopOffset += blockDelta;
+
+    for (ColumnRectIterator it(*this); it.hasMore(); it.advance()) {
+        LayoutRect hitRect = result.rectForPoint(pointInContainer);
+        LayoutRect colRect = it.columnRect();
         colRect.moveBy(accumulatedOffset);
-        
-        if (colRect.intersects(result.rectForPoint(pointInContainer))) {
+        if (colRect.intersects(hitRect)) {
             // The point is inside this column.
             // Adjust accumulatedOffset to change where we hit test.
-        
-            LayoutSize offset = isHorizontal ? IntSize(currLogicalLeftOffset, currLogicalTopOffset) : LayoutSize(currLogicalTopOffset, currLogicalLeftOffset);
-            if (colInfo->progressionAxis() == ColumnInfo::BlockAxis) {
-                if (isHorizontal)
-                    offset.expand(0, colRect.y() - accumulatedOffset.y() - borderTop() - paddingTop());
-                else
-                    offset.expand(colRect.x() - accumulatedOffset.x() - borderLeft() - paddingLeft(), 0);
-            }
-
+            LayoutSize offset;
+            it.adjust(offset);
             LayoutPoint finalLocation = accumulatedOffset + offset;
-            if (result.isRectBasedTest() && !colRect.contains(result.rectForPoint(pointInContainer)))
-                hitTestContents(request, result, pointInContainer, finalLocation, hitTestAction);
-            else
+            if (!result.isRectBasedTest() || colRect.contains(hitRect))
                 return hitTestContents(request, result, pointInContainer, finalLocation, hitTestAction) || (hitTestAction == HitTestFloat && hitTestFloats(request, result, pointInContainer, finalLocation));
+
+            hitTestContents(request, result, pointInContainer, finalLocation, hitTestAction);
         }
     }
 
     return false;
+}
+
+void RenderBlock::adjustForColumnRect(LayoutSize& offset, const LayoutPoint& pointInContainer) const
+{
+    for (ColumnRectIterator it(*this); it.hasMore(); it.advance()) {
+        LayoutRect colRect = it.columnRect();
+        if (colRect.contains(pointInContainer)) {
+            it.adjust(offset);
+            return;
+        }
+    }
 }
 

trunk/Source/WebCore/rendering/RenderBlock.h

@@ -197 +197 @@
     void adjustRectForColumns(LayoutRect&) const;
     virtual void adjustForColumns(LayoutSize&, const LayoutPoint&) const;
+    void adjustForColumnRect(LayoutSize& offset, const LayoutPoint& pointInContainer) const;
 
     void addContinuationWithOutline(RenderInline*);

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -1394 +1394 @@
     } else
         fixed |= isFixedPos;
-    
-    RenderObject* o = container();
-    if (!o)
-        return;
-
-    o->mapAbsoluteToLocalPoint(fixed, useTransforms, transformState);
-
-    LayoutSize containerOffset = offsetFromContainer(o, LayoutPoint());
-
-    bool preserve3D = useTransforms && (o->style()->preserves3D() || style()->preserves3D());
-    if (useTransforms && shouldUseTransformFromContainer(o)) {
-        TransformationMatrix t;
-        getTransformFromContainer(o, containerOffset, t);
-        transformState.applyTransform(t, preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
-    } else
-        transformState.move(containerOffset.width(), containerOffset.height(), preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
+
+    RenderBoxModelObject::mapAbsoluteToLocalPoint(fixed, useTransforms, transformState);
 }
 

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -38 +38 @@
 #include "RenderView.h"
 #include "Settings.h"
+#include "TransformState.h"
 #include <wtf/CurrentTime.h>
 
@@ -2718 +2719 @@
 }
 
+void RenderBoxModelObject::mapAbsoluteToLocalPoint(bool fixed, bool useTransforms, TransformState& transformState) const
+{
+    // We don't expect absoluteToLocal() to be called during layout (yet)
+    ASSERT(!view() || !view()->layoutStateEnabled());
+
+    RenderObject* o = container();
+    if (!o)
+        return;
+
+    o->mapAbsoluteToLocalPoint(fixed, useTransforms, transformState);
+
+    LayoutSize containerOffset = offsetFromContainer(o, LayoutPoint());
+
+    if (style()->position() != AbsolutePosition && style()->position() != FixedPosition && o->hasColumns()) {
+        RenderBlock* block = static_cast<RenderBlock*>(o);
+        LayoutPoint point(roundedLayoutPoint(transformState.mappedPoint()));
+        point -= containerOffset;
+        block->adjustForColumnRect(containerOffset, point);
+    }
+
+    bool preserve3D = useTransforms && (o->style()->preserves3D() || style()->preserves3D());
+    if (useTransforms && shouldUseTransformFromContainer(o)) {
+        TransformationMatrix t;
+        getTransformFromContainer(o, containerOffset, t);
+        transformState.applyTransform(t, preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
+    } else
+        transformState.move(containerOffset.width(), containerOffset.height(), preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
+}
+
 } // namespace WebCore

trunk/Source/WebCore/rendering/RenderBoxModelObject.h

@@ -125 +125 @@
     virtual LayoutUnit lineHeight(bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const = 0;
     virtual LayoutUnit baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const = 0;
+
+    virtual void mapAbsoluteToLocalPoint(bool fixed, bool useTransforms, TransformState&) const OVERRIDE;
 
     // Called by RenderObject::willBeDestroyed() and is the only way layers should ever be destroyed

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -1162 +1162 @@
 }
 
-void RenderInline::mapAbsoluteToLocalPoint(bool fixed, bool useTransforms, TransformState& transformState) const
-{
-    // We don't expect this function to be called during layout.
-    ASSERT(!view() || !view()->layoutStateEnabled());
-
-    RenderObject* o = container();
-    if (!o)
-        return;
-
-    o->mapAbsoluteToLocalPoint(fixed, useTransforms, transformState);
-
-    LayoutSize containerOffset = offsetFromContainer(o, LayoutPoint());
-
-    bool preserve3D = useTransforms && (o->style()->preserves3D() || style()->preserves3D());
-    if (useTransforms && shouldUseTransformFromContainer(o)) {
-        TransformationMatrix t;
-        getTransformFromContainer(o, containerOffset, t);
-        transformState.applyTransform(t, preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
-    } else
-        transformState.move(containerOffset.width(), containerOffset.height(), preserve3D ? TransformState::AccumulateTransform : TransformState::FlattenTransform);
-}
-
 void RenderInline::updateDragState(bool dragOn)
 {

trunk/Source/WebCore/rendering/RenderInline.h

@@ -132 +132 @@
 
     virtual void mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState&, bool* wasFixed = 0) const;
-    virtual void mapAbsoluteToLocalPoint(bool fixed, bool useTransforms, TransformState&) const;
 
     virtual VisiblePosition positionForPoint(const LayoutPoint&);