- Revision
- 167404
- Author
- hy...@apple.com
- Date
- 2014-04-16 17:27:34 -0700 (Wed, 16 Apr 2014)
Log Message
[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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (167403 => 167404)
--- trunk/Source/WebCore/ChangeLog 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/ChangeLog 2014-04-17 00:27:34 UTC (rev 167404)
@@ -1,3 +1,35 @@
+2014-04-16 David Hyatt <hy...@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 <d...@apple.com>
MediaDocument on iOS should be full page
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (167403 => 167404)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-04-17 00:27:34 UTC (rev 167404)
@@ -2406,7 +2406,7 @@
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 @@
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.
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp (167403 => 167404)
--- trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp 2014-04-17 00:27:34 UTC (rev 167404)
@@ -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 @@
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;
}
+
+}
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.h (167403 => 167404)
--- trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.h 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnFlowThread.h 2014-04-17 00:27:34 UTC (rev 167404)
@@ -94,6 +94,8 @@
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;
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp (167403 => 167404)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-04-17 00:27:34 UTC (rev 167404)
@@ -877,6 +877,101 @@
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";
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.h (167403 => 167404)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-04-16 23:59:13 UTC (rev 167403)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-04-17 00:27:34 UTC (rev 167404)
@@ -140,6 +140,8 @@
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);
