Changeset 123684 in webkit

Timestamp:

Jul 25, 2012 5:30:35 PM (12 years ago)

Author:

mitz@apple.com

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/multicol/hit-test-end-of-column-expected.txt (added)
• LayoutTests/fast/multicol/hit-test-end-of-column.html (added)
• LayoutTests/fast/multicol/hit-test-gap-block-axis-expected.txt (added)
• LayoutTests/fast/multicol/hit-test-gap-block-axis.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -4926 +4926 @@
         if (!firstRootBoxWithChildren)
             firstRootBoxWithChildren = root;
+
+        if (root->paginationStrut() && pointInLogicalContents.y() < root->logicalTop())
+            break;
+
         lastRootBoxWithChildren = root;
 
@@ -5248 +5252 @@
             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);
                 }
 
@@ -5273 +5284 @@
             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);
                 }