[webkit-changes] [119967] trunk

Mon, 11 Jun 2012 02:40:05 -0700

Title: [119967] trunk
Revision
119967
Author
[email protected]
Date
2012-06-11 02:38:48 -0700 (Mon, 11 Jun 2012)

Log Message

Rect-based hittesting doesn't work in tables.
https://bugs.webkit.org/show_bug.cgi?id=86605

Patch by Allan Sandfeld Jensen <[email protected]> on 2012-06-11
Reviewed by Julien Chaffraix.

Source/WebCore:

The existing code only performed a single binary lookup for the central
hit-test point. This meant area-based hit-testing did not work across
table-cell borders. All cells spanned by a hit-test area must be hit
tested.

This patch introduces three auxilary functions to help calculate rows
and columns spanned by an rect. These are intended to also be used by
repaint logic in a later patch.

For point-based hit-testing we maintain unchanged behavior and still
only hit test a single column in a single row.

Test: fast/dom/nodesFromRect-table.html

* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::logicalRectForWritingModeAndDirection):
(WebCore::RenderTableSection::spannedRows):
(WebCore::RenderTableSection::spannedColumns):
(WebCore::RenderTableSection::nodeAtPoint):
* rendering/RenderTableSection.h:
(RenderTableSection):

LayoutTests:

Test nodesFromRect on tables.

* fast/dom/nodesFromRect-table-expected.txt: Added.
* fast/dom/nodesFromRect-table.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (119966 => 119967)


--- trunk/LayoutTests/ChangeLog	2012-06-11 09:26:31 UTC (rev 119966)
+++ trunk/LayoutTests/ChangeLog	2012-06-11 09:38:48 UTC (rev 119967)
@@ -1,3 +1,15 @@
+2012-06-11  Allan Sandfeld Jensen  <[email protected]>
+
+        Rect-based hittesting doesn't work in tables.
+        https://bugs.webkit.org/show_bug.cgi?id=86605
+
+        Reviewed by Julien Chaffraix.
+
+        Test nodesFromRect on tables.
+
+        * fast/dom/nodesFromRect-table-expected.txt: Added.
+        * fast/dom/nodesFromRect-table.html: Added.
+
 2012-06-11  Szilard Ledan  <[email protected]>
 
         [Qt] Gardening, skip new failing tests.

Added: trunk/LayoutTests/fast/dom/nodesFromRect-table-expected.txt (0 => 119967)


--- trunk/LayoutTests/fast/dom/nodesFromRect-table-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/nodesFromRect-table-expected.txt	2012-06-11 09:38:48 UTC (rev 119967)
@@ -0,0 +1,38 @@
+Document::nodesFromRect : Rect-based hit-testing on tables - bug 86605
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Check area hits fully inside table cells
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+Check area hits across table border
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+Check area hits crossing neighbouring table cells
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+Check area hits crossing entire table cells
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+Check area hits on right-to-left table
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+Check area hits on flipped (tb-lr) table
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+

Added: trunk/LayoutTests/fast/dom/nodesFromRect-table.html (0 => 119967)


--- trunk/LayoutTests/fast/dom/nodesFromRect-table.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/nodesFromRect-table.html	2012-06-11 09:38:48 UTC (rev 119967)
@@ -0,0 +1,121 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Document::nodesFromRect : Rect-based hit-testing on tables - bug 86605</title>
+<style type="text/css">
+    #sandbox {
+        position: absolute;
+        left: 0px;
+        top: 0px;
+        width: 600px;
+        height: 800px;
+    }
+    #testtable {
+        margin: 100px;
+        width: 200px;
+        height: 200px;
+    }
+    .rtl {
+        direction: rtl;
+    }
+    .tblr {
+        -webkit-writing-mode: vertical-lr;
+    }
+</style>
+<script src=""
+<script src=""
+</head>
+
+<body>
+    <div id=sandbox>
+        <table id=testtable>
+            <tr id=tr1>
+                <td id=td11>
+                <td id=td12>
+                <td id=td13>
+                <td id=td14>
+            <tr id=tr2>
+                <td id=td21>
+                <td id=td22>
+                <td id=td23>
+                <td id=td24>
+            <tr id=tr3>
+                <td id=td31>
+                <td id=td32>
+                <td id=td33>
+                <td id=td34>
+            <tr id=tr4>
+                <td id=td41>
+                <td id=td42>
+                <td id=td43>
+                <td id=td44>
+        </table>
+    </div>
+
+    <script>
+        function runTest()
+        {
+            description(document.title);
+            var e = {};
+
+            // Set up shortcut access to elements
+            ['sandbox', 'testtable', 'tr1', 'tr2', 'tr3', 'tr4',
+             'td11', 'td12', 'td13', 'td14',
+             'td21', 'td22', 'td23', 'td24',
+             'td31', 'td32', 'td33', 'td34',
+             'td41', 'td42', 'td43', 'td44'].forEach(function(a) {
+                e[a] = document.getElementById(a);
+            });
+
+            window.scrollTo(0, 0);
+            debug('Check area hits fully inside table cells');
+            check(125, 125, 10, 10, 10, 10, [e.td11]);
+            check(275, 125, 10, 10, 10, 10, [e.td14]);
+            check(175, 175, 10, 10, 10, 10, [e.td22]);
+            check(125, 275, 10, 10, 10, 10, [e.td41]);
+
+            debug('Check area hits across table border');
+            check(90, 125, 10, 20, 10, 20, [e.td11, e.testtable, e.sandbox]);
+            check(275, 90, 20, 10, 20, 10, [e.td14, e.testtable, e.sandbox]);
+            check(310, 310, 20, 20, 20, 20, [e.td44, e.testtable, e.sandbox]);
+
+            debug('Check area hits crossing neighbouring table cells');
+            check(190, 175, 10, 20, 10, 20, [e.td22, e.td23, e.testtable]);
+            check(260, 275, 10, 20, 10, 20, [e.td43, e.td44, e.testtable]);
+            check(175, 190, 20, 10, 20, 10, [e.td22, e.td32, e.testtable]);
+            check(275, 260, 20, 10, 20, 10, [e.td34, e.td44, e.testtable]);
+            check(190, 190, 20, 20, 20, 20, [e.td22, e.td23, e.td32, e.td33, e.testtable]);
+
+            debug('Check area hits crossing entire table cells');
+            check(200, 175, 10, 90, 10, 90, [e.td21, e.td22, e.td23, e.td24, e.testtable]);
+            check(175, 200, 90, 10, 90, 10, [e.td12, e.td22, e.td32, e.td42, e.testtable]);
+            check(150, 150, 60, 60, 60, 60, [e.td11, e.td12, e.td13,
+                                             e.td21, e.td22, e.td23, 
+                                             e.td31, e.td32, e.td33, 
+                                             e.testtable, e.sandbox]);
+            check(125, 350, 120, 10, 10, 10, [e.td31, e.td41, e.testtable, e.sandbox]);
+            check(350, 150, 10, 10, 10, 120, [e.td13, e.td14, e.td23, e.td24, e.testtable, e.sandbox]);
+
+            debug('Check area hits on right-to-left table');
+            e['testtable'].setAttribute('class', 'rtl');
+            check(275, 125, 10, 10, 10, 10, [e.td11]);
+            check(175, 175, 10, 10, 10, 10, [e.td23]);
+            check(175, 190, 20, 10, 20, 10, [e.td23, e.td33, e.testtable]);
+
+            debug('Check area hits on flipped (tb-lr) table');
+            e['testtable'].setAttribute('class', 'tblr');
+            check(275, 125, 10, 10, 10, 10, [e.td41]);
+            check(125, 275, 10, 10, 10, 10, [e.td14]);
+            check(200, 175, 10, 90, 10, 90, [e.td12, e.td22, e.td32, e.td42, e.testtable]);
+
+        }
+
+        window._onload_ = runTest;
+    </script>
+
+    <p id='description'></p>
+    <span id="console"></span>
+    <script src=""
+</body>
+</html>
+

Modified: trunk/Source/WebCore/ChangeLog (119966 => 119967)


--- trunk/Source/WebCore/ChangeLog	2012-06-11 09:26:31 UTC (rev 119966)
+++ trunk/Source/WebCore/ChangeLog	2012-06-11 09:38:48 UTC (rev 119967)
@@ -1,3 +1,32 @@
+2012-06-11  Allan Sandfeld Jensen  <[email protected]>
+
+        Rect-based hittesting doesn't work in tables.
+        https://bugs.webkit.org/show_bug.cgi?id=86605
+
+        Reviewed by Julien Chaffraix.
+
+        The existing code only performed a single binary lookup for the central
+        hit-test point. This meant area-based hit-testing did not work across
+        table-cell borders. All cells spanned by a hit-test area must be hit
+        tested.
+
+        This patch introduces three auxilary functions to help calculate rows
+        and columns spanned by an rect. These are intended to also be used by
+        repaint logic in a later patch.
+
+        For point-based hit-testing we maintain unchanged behavior and still
+        only hit test a single column in a single row.
+
+        Test: fast/dom/nodesFromRect-table.html
+
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::logicalRectForWritingModeAndDirection):
+        (WebCore::RenderTableSection::spannedRows):
+        (WebCore::RenderTableSection::spannedColumns):
+        (WebCore::RenderTableSection::nodeAtPoint):
+        * rendering/RenderTableSection.h:
+        (RenderTableSection):
+
 2012-06-11  Min Qin  <[email protected]>
 
         Adding a flag to show fullscreen media controls in chromium

Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (119966 => 119967)


--- trunk/Source/WebCore/rendering/RenderTableSection.cpp	2012-06-11 09:26:31 UTC (rev 119966)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp	2012-06-11 09:38:48 UTC (rev 119967)
@@ -1026,6 +1026,23 @@
         cell->paint(paintInfo, cellPoint);
 }
 
+LayoutRect RenderTableSection::logicalRectForWritingModeAndDirection(const LayoutRect& rect) const
+{
+    LayoutRect tableAlignedRect(rect);
+
+    flipForWritingMode(tableAlignedRect);
+
+    if (!style()->isHorizontalWritingMode())
+        tableAlignedRect = tableAlignedRect.transposedRect();
+
+    const Vector<int>& columnPos = table()->columnPositions();
+    if (!style()->isLeftToRightDirection())
+        tableAlignedRect.setX(columnPos[columnPos.size() - 1] - tableAlignedRect.maxX());
+
+    return tableAlignedRect;
+}
+
+// FIXME: Use spannedRows internally, see https://bugs.webkit.org/show_bug.cgi?id=88066
 CellSpan RenderTableSection::dirtiedRows(const LayoutRect& damageRect) const
 {
     if (m_forceSlowPaintPathWithOverflowingCell) 
@@ -1054,6 +1071,7 @@
 }
 
 
+// FIXME: Use spannedColumns internally, see https://bugs.webkit.org/show_bug.cgi?id=88066
 CellSpan RenderTableSection::dirtiedColumns(const LayoutRect& damageRect) const
 {
     if (m_forceSlowPaintPathWithOverflowingCell) 
@@ -1080,6 +1098,57 @@
     return CellSpan(startCol, endCol);
 }
 
+CellSpan RenderTableSection::spannedRows(const LayoutRect& flippedRect) const
+{
+    // Find the first row that starts after rect top.
+    // FIXME: Upper_bound might not be the correct algorithm here since it might skip empty rows, but it is
+    // consistent with behavior in the former point based hit-testing (but inconsistent with spannedColumns).
+    unsigned nextRow = std::upper_bound(m_rowPos.begin(), m_rowPos.end(), flippedRect.y()) - m_rowPos.begin();
+
+    if (nextRow == m_rowPos.size())
+        return CellSpan(m_rowPos.size() - 1, m_rowPos.size() - 1); // After all rows.
+
+    unsigned startRow = nextRow > 0 ? nextRow - 1 : 0;
+
+    // Find the first row that starts after rect bottom.
+    unsigned endRow;
+    if (m_rowPos[nextRow] >= flippedRect.maxY())
+        endRow = nextRow;
+    else {
+        endRow = std::upper_bound(m_rowPos.begin() + nextRow, m_rowPos.end(), flippedRect.maxY()) - m_rowPos.begin();
+        if (endRow == m_rowPos.size())
+            endRow = m_rowPos.size() - 1;
+    }
+
+    return CellSpan(startRow, endRow);
+}
+
+CellSpan RenderTableSection::spannedColumns(const LayoutRect& flippedRect) const
+{
+    const Vector<int>& columnPos = table()->columnPositions();
+
+    // Find the first columnt that starts after rect left.
+    unsigned nextColumn = std::lower_bound(columnPos.begin(), columnPos.end(), flippedRect.x()) - columnPos.begin();
+
+    if (nextColumn == columnPos.size())
+        return CellSpan(columnPos.size() - 1, columnPos.size() - 1); // After all columns.
+
+    unsigned startColumn = nextColumn > 0 ? nextColumn - 1 : 0;
+
+    // Find the first row that starts after rect right.
+    unsigned endColumn;
+    if (columnPos[nextColumn] >= flippedRect.maxX())
+        endColumn = nextColumn;
+    else {
+        endColumn = std::lower_bound(columnPos.begin() + nextColumn, columnPos.end(), flippedRect.maxX()) - columnPos.begin();
+        if (endColumn == columnPos.size())
+            endColumn = columnPos.size() - 1;
+    }
+
+    return CellSpan(startColumn, endColumn);
+}
+
+
 void RenderTableSection::paintObject(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     PaintPhase paintPhase = paintInfo.phase;
@@ -1344,52 +1413,41 @@
         return false;
     }
 
-    LayoutPoint location = pointInContainer - toLayoutSize(adjustedLocation);
-    if (style()->isFlippedBlocksWritingMode()) {
-        if (style()->isHorizontalWritingMode())
-            location.setY(height() - location.y());
-        else
-            location.setX(width() - location.x());
-    }
-
-    LayoutUnit offsetInColumnDirection = style()->isHorizontalWritingMode() ? location.y() : location.x();
-    
     recalcCellsIfNeeded();
 
-    // Find the first row that starts after offsetInColumnDirection.
-    unsigned nextRow = std::upper_bound(m_rowPos.begin(), m_rowPos.end(), offsetInColumnDirection) - m_rowPos.begin();
-    if (nextRow == m_rowPos.size())
-        return false;
-    // Now set hitRow to the index of the hit row, or 0.
-    unsigned hitRow = nextRow > 0 ? nextRow - 1 : 0;
+    LayoutRect hitTestRect = result.rectForPoint(pointInContainer);
+    hitTestRect.moveBy(-adjustedLocation);
 
-    Vector<int>& columnPos = table()->columnPositions();
-    LayoutUnit offsetInRowDirection = style()->isHorizontalWritingMode() ? location.x() : location.y();
-    if (!style()->isLeftToRightDirection())
-        offsetInRowDirection = columnPos[columnPos.size() - 1] - offsetInRowDirection;
+    LayoutRect tableAlignedRect = logicalRectForWritingModeAndDirection(hitTestRect);
+    CellSpan rowSpan = spannedRows(tableAlignedRect);
+    CellSpan columnSpan = spannedColumns(tableAlignedRect);
 
-    unsigned nextColumn = std::lower_bound(columnPos.begin(), columnPos.end(), offsetInRowDirection) - columnPos.begin();
-    if (nextColumn == columnPos.size())
-        return false;
-    unsigned hitColumn = nextColumn > 0 ? nextColumn - 1 : 0;
+    // Now iterate over the spanned rows and columns.
+    for (unsigned hitRow = rowSpan.start(); hitRow < rowSpan.end(); ++hitRow) {
+        for (unsigned hitColumn = columnSpan.start(); hitColumn < columnSpan.end(); ++hitColumn) {
+            CellStruct& current = cellAt(hitRow, hitColumn);
 
-    CellStruct& current = cellAt(hitRow, hitColumn);
+            // If the cell is empty, there's nothing to do
+            if (!current.hasCells())
+                continue;
 
-    // If the cell is empty, there's nothing to do
-    if (!current.hasCells())
-        return false;
-
-    for (unsigned i = current.cells.size() ; i; ) {
-        --i;
-        RenderTableCell* cell = current.cells[i];
-        LayoutPoint cellPoint = flipForWritingModeForChild(cell, adjustedLocation);
-        if (static_cast<RenderObject*>(cell)->nodeAtPoint(request, result, pointInContainer, cellPoint, action)) {
-            updateHitTestResult(result, toLayoutPoint(pointInContainer - cellPoint));
-            return true;
+            for (unsigned i = current.cells.size() ; i; ) {
+                --i;
+                RenderTableCell* cell = current.cells[i];
+                LayoutPoint cellPoint = flipForWritingModeForChild(cell, adjustedLocation);
+                if (static_cast<RenderObject*>(cell)->nodeAtPoint(request, result, pointInContainer, cellPoint, action)) {
+                    updateHitTestResult(result, toLayoutPoint(pointInContainer - cellPoint));
+                    return true;
+                }
+            }
+            if (!result.isRectBasedTest())
+                break;
         }
+        if (!result.isRectBasedTest())
+            break;
     }
+
     return false;
-
 }
 
 void RenderTableSection::removeCachedCollapsedBorders(const RenderTableCell* cell)

Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (119966 => 119967)


--- trunk/Source/WebCore/rendering/RenderTableSection.h	2012-06-11 09:26:31 UTC (rev 119966)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h	2012-06-11 09:38:48 UTC (rev 119967)
@@ -227,9 +227,17 @@
     CellSpan fullTableRowSpan() const { return CellSpan(0, m_grid.size()); }
     CellSpan fullTableColumnSpan() const { return CellSpan(0, table()->columns().size()); }
 
+    // Flip the rect so it aligns with the coordinates used by the rowPos and columnPos vectors.
+    LayoutRect logicalRectForWritingModeAndDirection(const LayoutRect&) const;
+
     CellSpan dirtiedRows(const LayoutRect& repaintRect) const;
     CellSpan dirtiedColumns(const LayoutRect& repaintRect) const;
 
+    // These two functions take a rectangle as input that has been flipped by logicalRectForWritingModeAndDirection.
+    // The returned span of rows or columns is end-exclusive, and empty if start==end.
+    CellSpan spannedRows(const LayoutRect& flippedRect) const;
+    CellSpan spannedColumns(const LayoutRect& flippedRect) const;
+
     void setLogicalPositionForCell(RenderTableCell*, unsigned effectiveColumn) const;
 
     RenderObjectChildList m_children;

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to