[New Multicolumn] Selection gets confused when the mouse is in the column gaps.
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Apr 2014 00:27:34 +0000 (00:27 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Apr 2014 00:27:34 +0000 (00:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131739

Reviewed by Enrica Casucci.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::selectionGaps):
Make sure not to paint selection gaps. This matches the old multi-column behavior. Even though
selection gaps *nearly* work with the new multi-column code, I am disabling them so that we
can turn them on without visual regressions.

(WebCore::isChildHitTestCandidate):
Don't allow in-flow RenderFlowThreads to be descended into from positionForPoint. We always want
to look only at the spanners and at the sets.

* rendering/RenderMultiColumnFlowThread.cpp:
(WebCore::RenderMultiColumnFlowThread::nodeAtPoint):
* rendering/RenderMultiColumnFlowThread.h:
Override nodeAtPoint to disallow the RenderMultiColumnFlowThread from being considered for hit
testing when no DOM node is found. It's better to just let RenderBlock's positionForPoint run
to drill back down into the appropriate column set.

* rendering/RenderMultiColumnSet.cpp:
(WebCore::RenderMultiColumnSet::positionForPoint):
Implement positionForPoint for RenderMultiColumnSets. This is a straight-up port of the
old multi-column code's adjustPointToColumnContents function.

* rendering/RenderMultiColumnSet.h:
Add override of positionForPoint.

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp
Source/WebCore/rendering/RenderMultiColumnFlowThread.h
Source/WebCore/rendering/RenderMultiColumnSet.cpp
Source/WebCore/rendering/RenderMultiColumnSet.h

index 277c08d..0273a1e 100644 (file)
@@ -1,3 +1,35 @@
+2014-04-16  David Hyatt  <hyatt@apple.com>
+
+        [New Multicolumn] Selection gets confused when the mouse is in the column gaps.
+        https://bugs.webkit.org/show_bug.cgi?id=131739
+
+        Reviewed by Enrica Casucci.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::selectionGaps):
+        Make sure not to paint selection gaps. This matches the old multi-column behavior. Even though
+        selection gaps *nearly* work with the new multi-column code, I am disabling them so that we
+        can turn them on without visual regressions.
+        
+        (WebCore::isChildHitTestCandidate):
+        Don't allow in-flow RenderFlowThreads to be descended into from positionForPoint. We always want
+        to look only at the spanners and at the sets.
+
+        * rendering/RenderMultiColumnFlowThread.cpp:
+        (WebCore::RenderMultiColumnFlowThread::nodeAtPoint):
+        * rendering/RenderMultiColumnFlowThread.h:
+        Override nodeAtPoint to disallow the RenderMultiColumnFlowThread from being considered for hit
+        testing when no DOM node is found. It's better to just let RenderBlock's positionForPoint run
+        to drill back down into the appropriate column set.
+
+        * rendering/RenderMultiColumnSet.cpp:
+        (WebCore::RenderMultiColumnSet::positionForPoint):
+        Implement positionForPoint for RenderMultiColumnSets. This is a straight-up port of the
+        old multi-column code's adjustPointToColumnContents function.
+
+        * rendering/RenderMultiColumnSet.h:
+        Add override of positionForPoint.
+
 2014-04-16  Dean Jackson  <dino@apple.com>
 
         MediaDocument on iOS should be full page
index ab00bac..cbdfa72 100644 (file)
@@ -2406,7 +2406,7 @@ GapRects RenderBlock::selectionGaps(RenderBlock& rootBlock, const LayoutPoint& r
     if (!isRenderBlockFlow()) // FIXME: Make multi-column selection gap filling work someday.
         return result;
 
-    if (hasColumns() || hasTransform() || style().columnSpan()) {
+    if (hasColumns() || hasTransform() || style().columnSpan() || isInFlowRenderFlowThread()) {
         // FIXME: We should learn how to gap fill multiple columns and transforms eventually.
         lastLogicalTop = blockDirectionOffset(rootBlock, offsetFromRootBlock) + logicalHeight();
         lastLogicalLeft = logicalLeftSelectionOffset(rootBlock, logicalHeight(), cache);
@@ -3178,7 +3178,7 @@ VisiblePosition RenderBlock::positionForPointWithInlineChildren(const LayoutPoin
 
 static inline bool isChildHitTestCandidate(const RenderBox& box)
 {
-    return box.height() && box.style().visibility() == VISIBLE && !box.isFloatingOrOutOfFlowPositioned();
+    return box.height() && box.style().visibility() == VISIBLE && !box.isFloatingOrOutOfFlowPositioned() && !box.isInFlowRenderFlowThread();
 }
 
 // Valid candidates in a FlowThread must be rendered by the region.
index a8b0b20..3481262 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "RenderMultiColumnFlowThread.h"
 
+#include "HitTestResult.h"
 #include "LayoutState.h"
 #include "RenderMultiColumnSet.h"
 #include "RenderMultiColumnSpannerPlaceholder.h"
@@ -618,4 +619,16 @@ bool RenderMultiColumnFlowThread::isPageLogicalHeightKnown() const
     return false;
 }
 
+bool RenderMultiColumnFlowThread::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction hitTestAction)
+{
+    // You cannot be inside an in-flow RenderFlowThread without a corresponding DOM node. It's better to
+    // just let the ancestor figure out where we are instead.
+    if (hitTestAction == HitTestBlockBackground)
+        return false;
+    bool inside = RenderFlowThread::nodeAtPoint(request, result, locationInContainer, accumulatedOffset, hitTestAction);
+    if (inside && !result.innerNode())
+        return false;
+    return inside;
+}
+
 }
index a656115..c77e18e 100644 (file)
@@ -94,6 +94,8 @@ public:
     
     virtual RenderRegion* mapFromFlowToRegion(TransformState&) const override;
     
+    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;
+
 private:
     virtual const char* renderName() const override;
     virtual void addRegionToThread(RenderRegion*) override;
index ba70db6..a2326e7 100644 (file)
@@ -877,6 +877,101 @@ void RenderMultiColumnSet::addOverflowFromChildren()
         addVisualOverflow(lastRect);
 }
 
+VisiblePosition RenderMultiColumnSet::positionForPoint(const LayoutPoint& physicalPoint)
+{
+    // Determine which columns we intersect.
+    LayoutUnit colGap = columnGap();
+    LayoutUnit halfColGap = colGap / 2;
+    LayoutPoint columnPoint(columnRectAt(0).location());
+    LayoutUnit logicalOffset = 0;
+    
+    bool progressionIsInline = multiColumnFlowThread()->progressionIsInline();
+
+    LayoutPoint point = physicalPoint;
+    
+    for (unsigned i = 0; i < columnCount(); i++) {
+        // Add in half the column gap to the left and right of the rect.
+        LayoutRect colRect = columnRectAt(i);
+        flipForWritingMode(colRect);
+        if (isHorizontalWritingMode() == progressionIsInline) {
+            LayoutRect gapAndColumnRect(colRect.x() - halfColGap, colRect.y(), colRect.width() + colGap, colRect.height());
+            if (point.x() >= gapAndColumnRect.x() && point.x() < gapAndColumnRect.maxX()) {
+                if (progressionIsInline) {
+                    // FIXME: The clamping that follows is not completely right for right-to-left
+                    // content.
+                    // Clamp everything above the column to its top left.
+                    if (point.y() < gapAndColumnRect.y())
+                        point = gapAndColumnRect.location();
+                    // Clamp everything below the column to the next column's top left. If there is
+                    // no next column, this still maps to just after this column.
+                    else if (point.y() >= gapAndColumnRect.maxY()) {
+                        point = gapAndColumnRect.location();
+                        point.move(0, gapAndColumnRect.height());
+                    }
+                } else {
+                    if (point.x() < colRect.x())
+                        point.setX(colRect.x());
+                    else if (point.x() >= colRect.maxX())
+                        point.setX(colRect.maxX() - 1);
+                }
+
+                // We're inside the column. Translate the x and y into our column coordinate space.
+                if (progressionIsInline)
+                    point.move(columnPoint.x() - colRect.x(), (!style().isFlippedBlocksWritingMode() ? logicalOffset : -logicalOffset));
+                else
+                    point.move((!style().isFlippedBlocksWritingMode() ? logicalOffset : -logicalOffset) - colRect.x() + borderLeft() + paddingLeft(), 0);
+                
+                LayoutRect portion = flowThreadPortionRect();
+                flipForWritingMode(portion);
+                point.move(isHorizontalWritingMode() ? LayoutUnit() : portion.x(), isHorizontalWritingMode() ? portion.y() : LayoutUnit());
+                return multiColumnFlowThread()->positionForPoint(point);
+            }
+
+            // Move to the next position.
+            logicalOffset += progressionIsInline ? colRect.height() : colRect.width();
+        } else {
+            LayoutRect gapAndColumnRect(colRect.x(), colRect.y() - halfColGap, colRect.width(), colRect.height() + colGap);
+            if (point.y() >= gapAndColumnRect.y() && point.y() < gapAndColumnRect.maxY()) {
+                if (progressionIsInline) {
+                    // FIXME: The clamping that follows is not completely right for right-to-left
+                    // content.
+                    // Clamp everything above the column to its top left.
+                    if (point.x() < gapAndColumnRect.x())
+                        point = gapAndColumnRect.location();
+                    // Clamp everything below the column to the next column's top left. If there is
+                    // no next column, this still maps to just after this column.
+                    else if (point.x() >= gapAndColumnRect.maxX()) {
+                        point = gapAndColumnRect.location();
+                        point.move(gapAndColumnRect.width(), 0);
+                    }
+                } else {
+                    if (point.y() < colRect.y())
+                        point.setY(colRect.y());
+                    else if (point.y() >= colRect.maxY())
+                        point.setY(colRect.maxY() - 1);
+                }
+
+                // We're inside the column. Translate the x and y into our column coordinate space.
+                if (progressionIsInline)
+                    point.move((!style().isFlippedBlocksWritingMode() ? logicalOffset : -logicalOffset), columnPoint.y() - colRect.y());
+                else
+                    point.move(0, (!style().isFlippedBlocksWritingMode() ? logicalOffset : -logicalOffset) - colRect.y() + borderTop() + paddingTop());
+                
+                LayoutRect portion = flowThreadPortionRect();
+                flipForWritingMode(portion);
+                point.move(isHorizontalWritingMode() ? LayoutUnit() : portion.x(), isHorizontalWritingMode() ? portion.y() : LayoutUnit());
+                
+                return multiColumnFlowThread()->positionForPoint(point);
+            }
+
+            // Move to the next position.
+            logicalOffset += progressionIsInline ? colRect.width() : colRect.height();
+        }
+    }
+
+    return VisiblePosition();
+}
+
 const char* RenderMultiColumnSet::renderName() const
 {    
     return "RenderMultiColumnSet";
index 83954b5..da0077e 100644 (file)
@@ -140,6 +140,8 @@ private:
 
     virtual void adjustRegionBoundsFromFlowThreadPortionRect(const LayoutPoint& layerOffset, LayoutRect& regionBounds) override;
 
+    virtual VisiblePosition positionForPoint(const LayoutPoint&) override;
+
     virtual const char* renderName() const;
     
     void paintColumnRules(PaintInfo&, const LayoutPoint& paintOffset);