LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Apr 2007 01:25:35 +0000 (01:25 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Apr 2007 01:25:35 +0000 (01:25 +0000)
        Reviewed by harrison

        <rdar://problem/5126166>
        Deleting selection starting at before ToDo checkbox to end of line, inserts a BR in the subsequent ToDo

        * editing/deleting/5126166-expected.checksum: Added.
        * editing/deleting/5126166-expected.png: Added.
        * editing/deleting/5126166-expected.txt: Added.
        * editing/deleting/5126166.html: Added.

WebCore:

        Reviewed by harrison

        <rdar://problem/5126166>
        Deleting selection starting at before ToDo checkbox to end of line, inserts a BR in the subsequent ToDo

        If a selection ends in a table cell, we shouldn't perform
        a merge after deleting that selection.  We have code in
        place to prevent those merges, but it failed here.

        It fails because the end of the selection was [tableCell, 0],
        (normally not a valid VisiblePosition, but valid here
        because the table cell is empty).  We prevent the merge
        if the node of the position at the end of the selection
        has an enclosingTableCell.  Even though [tableCell, 0] has
        an enclosing table cell, the node tableCell doesn't, so
        the check fails.

        Fixed this by changing enclosingTableCell to take in
        a position, instead of a node.  The other enclosing element
        getters should be changed in this way as well.

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializePositionData):
        Call the new enclosingTableCell.
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::shouldMerge): Ditto.
        * editing/htmlediting.cpp:
        (WebCore::enclosingTableCell): Take in a position instead
        of a node.
        * editing/htmlediting.h:

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

LayoutTests/ChangeLog
LayoutTests/editing/deleting/5126166-expected.checksum [new file with mode: 0644]
LayoutTests/editing/deleting/5126166-expected.png [new file with mode: 0644]
LayoutTests/editing/deleting/5126166-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/5126166.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp
WebCore/editing/htmlediting.cpp
WebCore/editing/htmlediting.h

index a40610d..c2f1c8e 100644 (file)
@@ -1,3 +1,15 @@
+2007-04-11  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+        
+        <rdar://problem/5126166> 
+        Deleting selection starting at before ToDo checkbox to end of line, inserts a BR in the subsequent ToDo
+
+        * editing/deleting/5126166-expected.checksum: Added.
+        * editing/deleting/5126166-expected.png: Added.
+        * editing/deleting/5126166-expected.txt: Added.
+        * editing/deleting/5126166.html: Added.
+
 2007-04-10  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by harrison
diff --git a/LayoutTests/editing/deleting/5126166-expected.checksum b/LayoutTests/editing/deleting/5126166-expected.checksum
new file mode 100644 (file)
index 0000000..9d1fa5b
--- /dev/null
@@ -0,0 +1 @@
+aa28d039d8db71a249e54fdef5173a38
\ No newline at end of file
diff --git a/LayoutTests/editing/deleting/5126166-expected.png b/LayoutTests/editing/deleting/5126166-expected.png
new file mode 100644 (file)
index 0000000..a181210
Binary files /dev/null and b/LayoutTests/editing/deleting/5126166-expected.png differ
diff --git a/LayoutTests/editing/deleting/5126166-expected.txt b/LayoutTests/editing/deleting/5126166-expected.txt
new file mode 100644 (file)
index 0000000..52b2d0a
--- /dev/null
@@ -0,0 +1,26 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 779x36
+          text run at (0,0) width 414: "This tests deleting a selection that ends inside an empty table cell. "
+          text run at (414,0) width 365: "No merging should happen, only editable selected content"
+          text run at (0,18) width 124: "should be removed."
+      RenderBlock {DIV} at (0,52) size 784x56
+        RenderTable {TABLE} at (0,0) size 16x28 [border: (1px outset #808080)]
+          RenderTableSection {TBODY} at (1,1) size 14x26
+            RenderTableRow {TR} at (0,2) size 14x22
+              RenderTableCell {TD} at (2,11) size 4x4 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+              RenderTableCell {TD} at (8,2) size 4x22 [border: (1px inset #808080)] [r=0 c=1 rs=1 cs=1]
+                RenderBR {BR} at (2,2) size 0x18
+        RenderTable {TABLE} at (0,28) size 351x28 [border: (1px outset #808080)]
+          RenderTableSection {TBODY} at (1,1) size 349x26
+            RenderTableRow {TR} at (0,2) size 349x22
+              RenderTableCell {TD} at (2,2) size 4x22 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                RenderBR {BR} at (2,2) size 0x18
+              RenderTableCell {TD} at (8,2) size 339x22 [border: (1px inset #808080)] [r=0 c=1 rs=1 cs=1]
+                RenderText {#text} at (2,2) size 335x18
+                  text run at (2,2) width 335: "There should be one empty cell to the left of this one."
+caret: position 0 of child 0 {BR} of child 1 {TD} of child 0 {TR} of child 0 {TBODY} of child 1 {TABLE} of child 3 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/deleting/5126166.html b/LayoutTests/editing/deleting/5126166.html
new file mode 100644 (file)
index 0000000..0bd51de
--- /dev/null
@@ -0,0 +1,22 @@
+<body>
+<p>This tests deleting a selection that ends inside an empty table cell.  No merging should happen, only editable selected content should be removed.</p>
+<div contenteditable="true">
+<table border="1"><tr>
+    <td></td>
+    <td id="start">Editable</td>
+</tr></table>
+<table border="1"><tr>
+    <td id="end"></td>
+    <td>There should be one empty cell to the left of this one.</td>
+</tr></table>
+</div>
+
+<script>
+var sel = window.getSelection();
+var start = document.getElementById("start").firstChild;
+var end = document.getElementById("end");
+
+sel.setBaseAndExtent(start, 0, end, 0);
+document.execCommand("Delete");
+</script>
+</body>
index 1b6a527..a0f4f7d 100644 (file)
@@ -1,3 +1,36 @@
+2007-04-11  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+
+        <rdar://problem/5126166> 
+        Deleting selection starting at before ToDo checkbox to end of line, inserts a BR in the subsequent ToDo
+        
+        If a selection ends in a table cell, we shouldn't perform
+        a merge after deleting that selection.  We have code in
+        place to prevent those merges, but it failed here.
+        
+        It fails because the end of the selection was [tableCell, 0],
+        (normally not a valid VisiblePosition, but valid here
+        because the table cell is empty).  We prevent the merge
+        if the node of the position at the end of the selection 
+        has an enclosingTableCell.  Even though [tableCell, 0] has 
+        an enclosing table cell, the node tableCell doesn't, so 
+        the check fails.
+        
+        Fixed this by changing enclosingTableCell to take in
+        a position, instead of a node.  The other enclosing element
+        getters should be changed in this way as well.
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData):
+        Call the new enclosingTableCell.
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::shouldMerge): Ditto.
+        * editing/htmlediting.cpp:
+        (WebCore::enclosingTableCell): Take in a position instead
+        of a node.
+        * editing/htmlediting.h:
+
 2007-04-12  Mark Rowe  <mrowe@apple.com>
 
         Qt build fix.
index ef6fe11..1b69591 100644 (file)
@@ -132,8 +132,8 @@ void DeleteSelectionCommand::initializePositionData()
     m_startRoot = editableRootForPosition(start);
     m_endRoot = editableRootForPosition(end);
     
-    Node* startCell = enclosingTableCell(m_upstreamStart.node());
-    Node* endCell = enclosingTableCell(m_downstreamEnd.node());
+    Node* startCell = enclosingTableCell(m_upstreamStart);
+    Node* endCell = enclosingTableCell(m_downstreamEnd);
     // Don't move content out of a table cell.
     // FIXME: This isn't right.  A borderless table with two rows and a single column would appear as two paragraphs.
     if (endCell && endCell != startCell)
index 93c728c..785f66a 100644 (file)
@@ -338,7 +338,7 @@ bool ReplaceSelectionCommand::shouldMerge(const VisiblePosition& from, const Vis
     return !enclosingNodeOfType(fromNode, &isMailPasteAsQuotationNode) &&
            fromNodeBlock && (!fromNodeBlock->hasTagName(blockquoteTag) || isMailBlockquote(fromNodeBlock))  &&
            enclosingListChild(fromNode) == enclosingListChild(toNode) &&
-           enclosingTableCell(fromNode) == enclosingTableCell(toNode) &&
+           enclosingTableCell(from.deepEquivalent()) == enclosingTableCell(from.deepEquivalent()) &&
            // Don't merge to or from a position before or after a block because it would
            // be a no-op and cause infinite recursion.
            !isBlock(fromNode) && !isBlock(toNode);
index f86b773..9a1819d 100644 (file)
@@ -577,12 +577,14 @@ Node* enclosingNodeOfType(Node* node, bool (*nodeIsOfType)(Node*))
     return 0;
 }
 
-Node* enclosingTableCell(Node* node)
+Node* enclosingTableCell(const Position& p)
 {
-    if (!node)
+    if (p.isNull())
         return 0;
-        
-    for (Node* n = node->parentNode(); n; n = n->parentNode())
+    // Note: Should theoretically start with p.node()->parentNode() if p is a position 
+    // that internally means before or after p.node(), but we don't use position's like 
+    // that for table cells.
+    for (Node* n = p.node(); n; n = n->parentNode())
         if (n->renderer() && n->renderer()->isTableCell())
             return n;
             
index b69bb38..df90717 100644 (file)
@@ -103,7 +103,7 @@ Position positionOutsideContainingSpecialElement(const Position&, Node** contain
 
 Node* enclosingNodeWithTag(Node*, const QualifiedName&);
 Node* enclosingNodeOfType(Node*, bool (*nodeIsOfType)(Node*));
-Node* enclosingTableCell(Node*);
+Node* enclosingTableCell(const Position&);
 Node* enclosingEmptyListItem(const VisiblePosition&);
 bool isListElement(Node*);
 Node* enclosingList(Node*);