Reviewed by Chris
Fix for this bug:
<rdar://problem/3820349> REGRESSION (Mail): select all, delete does not always delete everything
* khtml/editing/htmlediting.cpp:
(khtml::DeleteSelectionCommand::startPositionForDelete): New helper that determines when to
expand the selection outwards when the selection is on the visible boundary of a root
editable element. This fixes the bug. Note that this function also contains a little code
I factored out of doApply: it also takes care of adjusting the selection in the smart delete case.
(khtml::DeleteSelectionCommand::endPositionForDelete): Ditto.
(khtml::DeleteSelectionCommand::doApply): Call new helpers. Refactored out the code as described.
* khtml/editing/htmlediting.h: Declare new helpers.
* layout-tests/editing/deleting/delete-select-all-001-expected.txt: Added.
* layout-tests/editing/deleting/delete-select-all-001.html: Added.
* layout-tests/editing/deleting/delete-select-all-002-expected.txt: Added.
* layout-tests/editing/deleting/delete-select-all-002.html: Added.
* layout-tests/editing/deleting/delete-select-all-003-expected.txt: Added.
* layout-tests/editing/deleting/delete-select-all-003.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@7890 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/editing/deleting/delete-select-all-001-expected.txt b/LayoutTests/editing/deleting/delete-select-all-001-expected.txt
new file mode 100644
index 0000000..ae0614d
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-001-expected.txt
@@ -0,0 +1,10 @@
+layer at (0,0) size 800x600
+ RenderCanvas 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 {DIV} at (0,0) size 784x28 [border: (2px solid #FF0000)]
+selection is CARET:
+start: position 0 of of root {DIV}
+upstream: position 0 of of root {DIV}
+downstream: position 1 of of root {DIV}
diff --git a/LayoutTests/editing/deleting/delete-select-all-001.html b/LayoutTests/editing/deleting/delete-select-all-001.html
new file mode 100644
index 0000000..2264839
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-001.html
@@ -0,0 +1,51 @@
+<html>
+<head>
+
+<style>
+.editing {
+ border: 2px solid red;
+ padding: 12px;
+ font-size: 24px;
+ word-wrap: break-word;
+ -khtml-nbsp-mode: space;
+ -khtml-line-break: after-white-space;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+ selectAllCommand();
+ deleteCommand();
+}
+
+</script>
+
+<title>Editing Test</title>
+
+<!--
+
+Expected results:
+
+All the content in the contenteditable div should be deleted,
+including the table structure.
+
+-->
+
+</head>
+<body>
+<div contenteditable id="root" class="editing">
+<table border="1">
+<tr><td id="test">One</td><td>Two</td><td>Three</td></tr>
+<tr><td>One</td><td>Two</td><td>Three</td></tr>
+<tr><td>One</td><td>Two</td><td>Three</td></tr>
+</table>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-select-all-002-expected.txt b/LayoutTests/editing/deleting/delete-select-all-002-expected.txt
new file mode 100644
index 0000000..214cc89
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-002-expected.txt
@@ -0,0 +1,10 @@
+layer at (0,0) size 800x600
+ RenderCanvas 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 {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
+selection is CARET:
+start: position 0 of of root {DIV}
+upstream: position 0 of of root {DIV}
+downstream: position 1 of of root {DIV}
diff --git a/LayoutTests/editing/deleting/delete-select-all-002.html b/LayoutTests/editing/deleting/delete-select-all-002.html
new file mode 100644
index 0000000..3e6f711
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-002.html
@@ -0,0 +1,48 @@
+<html>
+<head>
+
+<style>
+.editing {
+ border: 2px solid red;
+ padding: 12px;
+ font-size: 24px;
+ word-wrap: break-word;
+ -khtml-nbsp-mode: space;
+ -khtml-line-break: after-white-space;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+ selectAllCommand();
+ deleteCommand();
+}
+
+</script>
+
+<title>Editing Test</title>
+
+<!--
+
+Expected results:
+
+All the content in the contenteditable div should be deleted,
+including the "you can't see this." span.
+
+-->
+
+</head>
+<body>
+<div contenteditable id="root" class="editing">
+<span style="display: hidden">you can't see this.</span>
+<span id="test">you can see this.</span>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/deleting/delete-select-all-003-expected.txt b/LayoutTests/editing/deleting/delete-select-all-003-expected.txt
new file mode 100644
index 0000000..ae0614d
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-003-expected.txt
@@ -0,0 +1,10 @@
+layer at (0,0) size 800x600
+ RenderCanvas 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 {DIV} at (0,0) size 784x28 [border: (2px solid #FF0000)]
+selection is CARET:
+start: position 0 of of root {DIV}
+upstream: position 0 of of root {DIV}
+downstream: position 1 of of root {DIV}
diff --git a/LayoutTests/editing/deleting/delete-select-all-003.html b/LayoutTests/editing/deleting/delete-select-all-003.html
new file mode 100644
index 0000000..b1b2ab9
--- /dev/null
+++ b/LayoutTests/editing/deleting/delete-select-all-003.html
@@ -0,0 +1,47 @@
+<html>
+<head>
+
+<style>
+.editing {
+ border: 2px solid red;
+ padding: 12px;
+ font-size: 24px;
+ word-wrap: break-word;
+ -khtml-nbsp-mode: space;
+ -khtml-line-break: after-white-space;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+ selectAllCommand();
+ deleteCommand();
+}
+
+</script>
+
+<title>Editing Test</title>
+
+<!--
+
+Expected results:
+
+All the content in the contenteditable div should be deleted,
+including the unordered list element.
+
+-->
+
+</head>
+<body>
+<div contenteditable id="root" class="editing">
+<ul id="test"><li>one</li><li>two</li><li>three</li></ul>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index 7079498..b00055e 100644
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,5 +1,28 @@
2004-10-25 Ken Kocienda <kocienda@apple.com>
+ Reviewed by Chris
+
+ Fix for this bug:
+
+ <rdar://problem/3820349> REGRESSION (Mail): select all, delete does not always delete everything
+
+ * khtml/editing/htmlediting.cpp:
+ (khtml::DeleteSelectionCommand::startPositionForDelete): New helper that determines when to
+ expand the selection outwards when the selection is on the visible boundary of a root
+ editable element. This fixes the bug. Note that this function also contains a little code
+ I factored out of doApply: it also takes care of adjusting the selection in the smart delete case.
+ (khtml::DeleteSelectionCommand::endPositionForDelete): Ditto.
+ (khtml::DeleteSelectionCommand::doApply): Call new helpers. Refactored out the code as described.
+ * khtml/editing/htmlediting.h: Declare new helpers.
+ * layout-tests/editing/deleting/delete-select-all-001-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-select-all-001.html: Added.
+ * layout-tests/editing/deleting/delete-select-all-002-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-select-all-002.html: Added.
+ * layout-tests/editing/deleting/delete-select-all-003-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-select-all-003.html: Added.
+
+2004-10-25 Ken Kocienda <kocienda@apple.com>
+
Reviewed by me
Added some more editing layout tests.
diff --git a/WebCore/khtml/editing/htmlediting.cpp b/WebCore/khtml/editing/htmlediting.cpp
index bf2b51f..37b8852 100644
--- a/WebCore/khtml/editing/htmlediting.cpp
+++ b/WebCore/khtml/editing/htmlediting.cpp
@@ -1296,6 +1296,34 @@
}
}
+Position DeleteSelectionCommand::startPositionForDelete() const
+{
+ Position pos = m_selectionToDelete.start();
+ ASSERT(pos.node()->inDocument());
+
+ ElementImpl *rootElement = pos.node()->rootEditableElement();
+ Position rootStart = Position(rootElement, 0);
+ if (pos == VisiblePosition(rootStart).deepEquivalent())
+ pos = rootStart;
+ else if (m_smartDelete && pos.leadingWhitespacePosition().isNotNull())
+ pos = VisiblePosition(pos).previous().deepEquivalent();
+ return pos;
+}
+
+Position DeleteSelectionCommand::endPositionForDelete() const
+{
+ Position pos = m_selectionToDelete.end();
+ ASSERT(pos.node()->inDocument());
+
+ ElementImpl *rootElement = pos.node()->rootEditableElement();
+ Position rootEnd = Position(rootElement, rootElement ? rootElement->childNodeCount() : 0).equivalentDeepPosition();
+ if (pos == VisiblePosition(rootEnd).deepEquivalent())
+ pos = rootEnd;
+ else if (m_smartDelete && pos.leadingWhitespacePosition().isNotNull())
+ pos = VisiblePosition(pos).next().deepEquivalent();
+ return pos;
+}
+
void DeleteSelectionCommand::doApply()
{
// If selection has not been set to a custom selection when the command was created,
@@ -1306,21 +1334,12 @@
if (!m_selectionToDelete.isRange())
return;
- ASSERT(m_selectionToDelete.start().node()->inDocument());
- ASSERT(m_selectionToDelete.end().node()->inDocument());
-
- if (m_smartDelete) {
- if (!m_selectionToDelete.start().leadingWhitespacePosition().isNull()) {
- m_selectionToDelete.modify(Selection::EXTEND, Selection::LEFT, CHARACTER);
- } else if (!m_selectionToDelete.end().trailingWhitespacePosition().isNull()) {
- m_selectionToDelete.modify(Selection::EXTEND, Selection::RIGHT, CHARACTER);
- }
- }
-
- Position upstreamStart(m_selectionToDelete.start().upstream(StayInBlock));
- Position downstreamStart(m_selectionToDelete.start().downstream(StayInBlock));
- Position upstreamEnd(m_selectionToDelete.end().upstream(StayInBlock));
- Position downstreamEnd(m_selectionToDelete.end().downstream(StayInBlock));
+ Position start = startPositionForDelete();
+ Position end = endPositionForDelete();
+ Position upstreamStart(start.upstream(StayInBlock));
+ Position downstreamStart(start.downstream(StayInBlock));
+ Position upstreamEnd(end.upstream(StayInBlock));
+ Position downstreamEnd(end.downstream(StayInBlock));
Position endingPosition;
// Save away whitespace situation before doing any deletions
diff --git a/WebCore/khtml/editing/htmlediting.h b/WebCore/khtml/editing/htmlediting.h
index 6fadb20..5239e79 100644
--- a/WebCore/khtml/editing/htmlediting.h
+++ b/WebCore/khtml/editing/htmlediting.h
@@ -273,9 +273,9 @@
private:
virtual bool preservesTypingStyle() const;
- void deleteDownstreamWS(const DOM::Position &start);
- bool containsOnlyWhitespace(const DOM::Position &start, const DOM::Position &end);
void moveNodesAfterNode(DOM::NodeImpl *startNode, DOM::NodeImpl *dstNode);
+ DOM::Position startPositionForDelete() const;
+ DOM::Position endPositionForDelete() const;
khtml::Selection m_selectionToDelete;
bool m_hasSelectionToDelete;
