- fixed <rdar://problem/
4044347> REGRESSION (Mail): Control-K in particular message moves insertion point to previous line
Tweaked the deleting code, and added three new deleting layout tests to confirm the new code works.
* khtml/editing/htmlediting.cpp:
(khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Removed special case with comment that said it was
for the case where a "selection contains only a BR right after a block ended". This code was being triggered in
more cases than just that one, and in all the cases I tested, the general delete code works fine.
(khtml::DeleteSelectionCommand::handleGeneralDelete): Changed the code that decides whether to delete an entire
block to understand the case where the end block is outside the start block, but contains the start block.
In that case, we want to delete the entire block. Not deleting the block was causing us to delete just the <br>,
and not the enclosing <div> in the case in the bug.
* layout-tests/editing/deleting/delete-line-015-expected.txt: Added.
* layout-tests/editing/deleting/delete-line-015.html: Added.
* layout-tests/editing/deleting/delete-line-016-expected.txt: Added.
* layout-tests/editing/deleting/delete-line-016.html: Added.
* layout-tests/editing/deleting/delete-line-017-expected.txt: Added.
* layout-tests/editing/deleting/delete-line-017.html: Added.
* layout-tests/editing/style/smoosh-styles-002-expected.txt: Updated to improved results. With the code change, the deletion
now deletes more than it used to. The old results had an empty text node and <h1> element that were both 0-sized, and now
we delete both of those.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@8871
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
--- /dev/null
+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 {P} at (0,0) size 784x54
+ RenderText {TEXT} at (0,0) size 778x54
+ text run at (0,0) width 764: "This tests deletion of an empty line starting inside a <div> element, but ending outside that element (just before a <br>). If"
+ text run at (0,18) width 778: "the deletion is successful, the result should have two lines, and the insertion point should be on the second line, at the end of"
+ text run at (0,36) width 106: "the editable area."
+ RenderBlock {DIV} at (0,70) size 784x36
+ RenderBlock {DIV} at (0,0) size 784x18
+ RenderText {TEXT} at (0,0) size 7x18
+ text run at (0,0) width 7: "a"
+ RenderBlock (anonymous) at (0,18) size 784x18
+ RenderBR {BR} at (0,0) size 0x18
+selection is CARET:
+start: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
--- /dev/null
+<html>
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+ extendSelectionForwardByLineCommand();
+ deleteCommand();
+}
+</script>
+</head>
+
+<body>
+
+<p>This tests deletion of an empty line starting inside a <div> element,
+but ending outside that element (just before a <br>). If the deletion is
+successful, the result should have two lines, and the insertion point
+should be on the second line, at the end of the editable area.</p>
+
+<div contenteditable>
+<div>a</div><div id="test"><br></div><br>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
--- /dev/null
+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 {P} at (0,0) size 784x54
+ RenderText {TEXT} at (0,0) size 767x54
+ text run at (0,0) width 767: "This tests deletion of an empty line starting inside a <div> element, but ending outside that element (just before a <br>). In"
+ text run at (0,18) width 766: "this case, the <div> has no content inside it, but a min-height style prevents it from collapsing. If the deletion is successful,"
+ text run at (0,36) width 725: "the result should have two lines, and the insertion point should be on the second line, at the end of the editable area."
+ RenderBlock {DIV} at (0,70) size 784x36
+ RenderBlock {DIV} at (0,0) size 784x18
+ RenderText {TEXT} at (0,0) size 7x18
+ text run at (0,0) width 7: "a"
+ RenderBlock (anonymous) at (0,18) size 784x18
+ RenderBR {BR} at (0,0) size 0x18
+selection is CARET:
+start: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 2 {BR} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
--- /dev/null
+<html>
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+ extendSelectionForwardByLineCommand();
+ deleteCommand();
+}
+</script>
+</head>
+
+<body>
+
+<p>This tests deletion of an empty line starting inside a <div> element,
+but ending outside that element (just before a <br>). In this case, the
+<div> has no content inside it, but a min-height style prevents it from
+collapsing. If the deletion is successful, the result should have two lines,
+and the insertion point should be on the second line, at the end of the editable area.</p>
+
+<div contenteditable>
+<div>a</div><div id="test" style="min-height:50px"></div><br>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
--- /dev/null
+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 {P} at (0,0) size 784x36
+ RenderText {TEXT} at (0,0) size 781x36
+ text run at (0,0) width 781: "This tests deletion of an empty line which is a <br> element just after a <div> element. If the deletion is successful, the result"
+ text run at (0,18) width 695: "should have two lines, and the insertion point should be at the start of the second line, just before the letter \"b\"."
+ RenderBlock {DIV} at (0,52) size 784x36
+ RenderBlock {DIV} at (0,0) size 784x18
+ RenderText {TEXT} at (0,0) size 7x18
+ text run at (0,0) width 7: "a"
+ RenderBlock (anonymous) at (0,18) size 784x0
+ RenderBlock {DIV} at (0,18) size 784x18
+ RenderText {TEXT} at (0,0) size 8x18
+ text run at (0,0) width 8: "b"
+selection is CARET:
+start: position 0 of child 1 {TEXT} of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+upstream: position 0 of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 0 of child 1 {TEXT} of child 2 {DIV} of child 4 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
--- /dev/null
+<html>
+
+<head>
+<script src="../editing.js"></script>
+<script>
+function editingTest() {
+ moveSelectionForwardByLineCommand();
+ extendSelectionForwardByLineCommand();
+ deleteCommand();
+}
+</script>
+</head>
+
+<body>
+
+<p>This tests deletion of an empty line which is a <br> element just after
+a <div> element. If the deletion is successful, the result should have two lines,
+and the insertion point should be at the start of the second line, just before the letter "b".</p>
+
+<div contenteditable>
+<div id="test">a</div><br><div>b</div>
+</div>
+
+</body>
+
+<script>runEditingTest()</script>
+
+</html>
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 784x571
+ RenderBody {BODY} at (8,8) size 784x584
RenderBlock {DIV} at (0,0) size 784x165 [border: (2px solid #0000FF)]
RenderBlock {DIV} at (14,14) size 756x56
RenderText {TEXT} at (0,0) size 67x28
text run at (31,2) width 46: "cde"
RenderText {TEXT} at (77,8) size 24x28
text run at (77,8) width 24: "fg"
- RenderText {TEXT} at (0,0) size 0x0
- RenderBlock {H1} at (0,62) size 784x0
selection is CARET:
start: position 3 of child 1 {TEXT} of child 1 {B} of child 1 {SPAN} of child 2 {FONT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
upstream: position 3 of child 1 {TEXT} of child 1 {B} of child 1 {SPAN} of child 2 {FONT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
+2005-03-13 Darin Adler <darin@apple.com>
+
+ Reviewed by John and Ken.
+
+ - fixed <rdar://problem/4044347> REGRESSION (Mail): Control-K in particular message moves insertion point to previous line
+
+ Tweaked the deleting code, and added three new deleting layout tests to confirm the new code works.
+
+ * khtml/editing/htmlediting.cpp:
+ (khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Removed special case with comment that said it was
+ for the case where a "selection contains only a BR right after a block ended". This code was being triggered in
+ more cases than just that one, and in all the cases I tested, the general delete code works fine.
+ (khtml::DeleteSelectionCommand::handleGeneralDelete): Changed the code that decides whether to delete an entire
+ block to understand the case where the end block is outside the start block, but contains the start block.
+ In that case, we want to delete the entire block. Not deleting the block was causing us to delete just the <br>,
+ and not the enclosing <div> in the case in the bug.
+
+ * layout-tests/editing/deleting/delete-line-015-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-line-015.html: Added.
+ * layout-tests/editing/deleting/delete-line-016-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-line-016.html: Added.
+ * layout-tests/editing/deleting/delete-line-017-expected.txt: Added.
+ * layout-tests/editing/deleting/delete-line-017.html: Added.
+ * layout-tests/editing/style/smoosh-styles-002-expected.txt: Updated to improved results. With the code change, the deletion
+ now deletes more than it used to. The old results had an empty text node and <h1> element that were both 0-sized, and now
+ we delete both of those.
+
2005-03-13 Darin Adler <darin@apple.com>
- fixed <rdar://problem/4049172> REGRESSION (403-405): Gmail: text box in "Invite a friend" section overlaps other sections
* khtml/rendering/render_flow.cpp:
(RenderFlow::paintLines):
-<<<<<<< ChangeLog
2005-03-01 Chris Blumenberg <cblu@apple.com>
Fixed: <rdar://problem/4030669> smart delete does not delete spaces from pasted content
return true;
}
- // Check for special-case where the selection contains only a BR right after a block ended.
- bool downstreamEndIsBR = m_downstreamEnd.node()->id() == ID_BR;
- Position upstreamFromBR = m_downstreamEnd.upstream();
- Position downstreamFromStart = m_downstreamStart.downstream();
- bool startIsBRAfterBlock = downstreamEndIsBR && downstreamFromStart.node() == m_downstreamEnd.node() &&
- m_downstreamEnd.node()->enclosingBlockFlowElement() != upstreamFromBR.node()->enclosingBlockFlowElement();
- if (startIsBRAfterBlock) {
- removeNode(m_downstreamEnd.node());
- m_endingPosition = upstreamFromBR;
- m_mergeBlocksAfterDelete = false;
- return true;
- }
-
// Not a special-case delete per se, but we can detect that the merging of content between blocks
// should not be done.
if (upstreamStartIsBR && downstreamStartIsBR)
setStartNode(m_startNode->traverseNextNode());
}
else if (m_startBlock != m_endBlock && isStartOfBlock(VisiblePosition(m_upstreamStart, m_selectionToDelete.startAffinity()))) {
- if (!isStartOfBlock(visibleEnd) && endAtEndOfBlock) {
+ if (!m_startBlock->isAncestor(m_endBlock) && !isStartOfBlock(visibleEnd) && endAtEndOfBlock) {
// Delete all the children of the block, but not the block itself.
setStartNode(m_startBlock->firstChild());
startOffset = 0;