Hit testing in one column or in the gap between cloumns along the block axis can...
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jul 2012 00:30:35 +0000 (00:30 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jul 2012 00:30:35 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=92311

Reviewed by Anders Carlsson.

Source/WebCore:

Tests: fast/multicol/hit-test-end-of-column.html
       fast/multicol/hit-test-gap-block-axis.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::positionForPointWithInlineChildren): To prevent hits after the last
line on a given column from returning the next line in the next column, added a check if
the hit occurred within the pagination strut of a line. Covered by the first test.
(WebCore::RenderBlock::adjustPointToColumnContents): Added clamp-to-column logic for the
block-axis case. This prevents hits near the bottom of the top half of the gap from bleeding
into the top of the next column. Covered by the second test.

LayoutTests:

* fast/multicol/hit-test-end-of-column-expected.txt: Added.
* fast/multicol/hit-test-end-of-column.html: Added.
* fast/multicol/hit-test-gap-block-axis-expected.txt: Added.
* fast/multicol/hit-test-gap-block-axis.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/multicol/hit-test-end-of-column-expected.txt [new file with mode: 0644]
LayoutTests/fast/multicol/hit-test-end-of-column.html [new file with mode: 0644]
LayoutTests/fast/multicol/hit-test-gap-block-axis-expected.txt [new file with mode: 0644]
LayoutTests/fast/multicol/hit-test-gap-block-axis.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index 0f11629..357bcca 100644 (file)
@@ -1,3 +1,15 @@
+2012-07-25  Dan Bernstein  <mitz@apple.com>
+
+        Hit testing in one column or in the gap between cloumns along the block axis can return a result from the wrong column
+        https://bugs.webkit.org/show_bug.cgi?id=92311
+
+        Reviewed by Anders Carlsson.
+
+        * fast/multicol/hit-test-end-of-column-expected.txt: Added.
+        * fast/multicol/hit-test-end-of-column.html: Added.
+        * fast/multicol/hit-test-gap-block-axis-expected.txt: Added.
+        * fast/multicol/hit-test-gap-block-axis.html: Added.
+
 2012-07-25  Ryosuke Niwa  <rniwa@webkit.org>
 
         Improve the output of editing/pasteboard/paste-noscript-xhtml.xhtml
diff --git a/LayoutTests/fast/multicol/hit-test-end-of-column-expected.txt b/LayoutTests/fast/multicol/hit-test-end-of-column-expected.txt
new file mode 100644 (file)
index 0000000..3c29e3a
--- /dev/null
@@ -0,0 +1,2 @@
+Lorem ipsum dolor sit amet consectetur elit.
+PASS
diff --git a/LayoutTests/fast/multicol/hit-test-end-of-column.html b/LayoutTests/fast/multicol/hit-test-end-of-column.html
new file mode 100644 (file)
index 0000000..c8cb184
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<div id="target" style="
+    outline: dashed lightblue;
+    width: 400px;
+    -webkit-columns: 2;
+    -webkit-column-gap: 0;
+    height: 90px;
+    font: 20px ahem;
+">Lorem ipsum dolor sit amet consectetur elit.</div>
+<p id="result">
+    FAIL: Test did not run.
+</p>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    // Clicking below the last line in the first column should not select anything from the first
+    // line on the second column.
+    var target = document.getElementById("target");
+    var hitOffset = document.caretRangeFromPoint(target.offsetLeft + 45, target.offsetTop + 87).startOffset;
+    document.getElementById("result").innerText = hitOffset === 26 || hitOffset === 24 ? "PASS" : "FAIL: hit offset " + hitOffset + ".";
+</script>
diff --git a/LayoutTests/fast/multicol/hit-test-gap-block-axis-expected.txt b/LayoutTests/fast/multicol/hit-test-gap-block-axis-expected.txt
new file mode 100644 (file)
index 0000000..de6af97
--- /dev/null
@@ -0,0 +1,3 @@
+PASS
+
+Lorem ipsum dolor sit amet consectetur elit.
diff --git a/LayoutTests/fast/multicol/hit-test-gap-block-axis.html b/LayoutTests/fast/multicol/hit-test-gap-block-axis.html
new file mode 100644 (file)
index 0000000..a77f315
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<p id="result">
+    FAIL: Test did not run.
+</p>
+<div id="target" style="
+    outline: dashed lightblue;
+    width: 200px;
+    -webkit-column-axis: vertical;
+    -webkit-column-gap: 100px;
+    height: 90px;
+    font: 20px ahem;
+    color: rgba(0, 0, 0, 0.5);
+">Lorem ipsum dolor sit amet consectetur elit.</div>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    // Clicking in the gap after the first column should not select anything from the second column.
+    var target = document.getElementById("target");
+    var hitOffset = document.caretRangeFromPoint(target.offsetLeft + 45, target.offsetTop + 115).startOffset;
+    document.getElementById("result").innerText = hitOffset === 26 || hitOffset === 24 ? "PASS" : "FAIL: hit offset " + hitOffset + ".";
+</script>
index 1802460..9b6f939 100644 (file)
@@ -1,3 +1,21 @@
+2012-07-25  Dan Bernstein  <mitz@apple.com>
+
+        Hit testing in one column or in the gap between cloumns along the block axis can return a result from the wrong column
+        https://bugs.webkit.org/show_bug.cgi?id=92311
+
+        Reviewed by Anders Carlsson.
+
+        Tests: fast/multicol/hit-test-end-of-column.html
+               fast/multicol/hit-test-gap-block-axis.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::positionForPointWithInlineChildren): To prevent hits after the last
+        line on a given column from returning the next line in the next column, added a check if
+        the hit occurred within the pagination strut of a line. Covered by the first test.
+        (WebCore::RenderBlock::adjustPointToColumnContents): Added clamp-to-column logic for the
+        block-axis case. This prevents hits near the bottom of the top half of the gap from bleeding
+        into the top of the next column. Covered by the second test.
+
 2012-07-25  David Grogan  <dgrogan@chromium.org>
 
         IndexedDB: Make db.version return an integer if appropriate
index e00d131..545784e 100755 (executable)
@@ -4925,6 +4925,10 @@ VisiblePosition RenderBlock::positionForPointWithInlineChildren(const LayoutPoin
             continue;
         if (!firstRootBoxWithChildren)
             firstRootBoxWithChildren = root;
+
+        if (root->paginationStrut() && pointInLogicalContents.y() < root->logicalTop())
+            break;
+
         lastRootBoxWithChildren = root;
 
         // check if this root line box is located at this y coordinate
@@ -5247,16 +5251,23 @@ void RenderBlock::adjustPointToColumnContents(LayoutPoint& point) const
         if (isHorizontalWritingMode() == (colInfo->progressionAxis() == ColumnInfo::InlineAxis)) {
             LayoutRect gapAndColumnRect(colRect.x() - halfColGap, colRect.y(), colRect.width() + colGap, colRect.height());
             if (point.x() >= gapAndColumnRect.x() && point.x() < gapAndColumnRect.maxX()) {
-                // 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());
+                if (colInfo->progressionAxis() == ColumnInfo::InlineAxis) {
+                    // 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.
@@ -5272,16 +5283,23 @@ void RenderBlock::adjustPointToColumnContents(LayoutPoint& point) const
         } else {
             LayoutRect gapAndColumnRect(colRect.x(), colRect.y() - halfColGap, colRect.width(), colRect.height() + colGap);
             if (point.y() >= gapAndColumnRect.y() && point.y() < gapAndColumnRect.maxY()) {
-                // 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);
+                if (colInfo->progressionAxis() == ColumnInfo::InlineAxis) {
+                    // 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.