Fix for this bug:
<rdar://problem/
3890955> 8A314: Forward delete sometimes fails to delete the selected quoted text
* khtml/editing/htmlediting.cpp:
(khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Fixed bonehead coding mistake in the
check for one of the special cases being checked for in this function. The specific case
intends to check for a selection that is only a <br> after a block ends (as in </div><br>). If it
sees such markup, it deletes only the <br> and bails. However, this code would run in *any*
case where a selection ended in a <br> after a block and would not delete any part of the
selection preceding the <br>. Bad. I have tightened the check to see that only a <br> is
selected.
Fixing the bug above was accomplished with an additional call to DOM::Position::downstream. This
new use of the function exposed this bug:
<rdar://problem/
3907666> Incorrectly coded loop in Position::downstream can lead to infinite loop
* khtml/xml/dom_position.cpp:
(DOM::Position::downstream): I am ashamed of my first cut at this. Rewrote the loop so it does
not have this fatal flaw. It is a much better design as well.
* layout-tests/editing/deleting/delete-
3800834-fix-expected.txt: Changes made this test
have what I consider to be a better result. Going with it.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@8135
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
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 784x80 [border: (2px solid #FF0000)]
+ RenderBlock {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 756x28
RenderInline {SPAN} at (0,0) size 37x28
RenderText {TEXT} at (0,0) size 37x28
text run at (0,0) width 37: "Foo"
- RenderBR {BR} at (0,0) size 0x0
- RenderBlock (anonymous) at (14,66) size 756x0
- RenderBlock {BLOCKQUOTE} at (40,0) size 676x0
- RenderBlock (anonymous) at (14,66) size 756x0
+ RenderBlock (anonymous) at (14,42) size 756x0
+ RenderBlock (anonymous) at (14,42) size 756x0
RenderInline {SPAN} at (0,0) size 0x0
RenderText {TEXT} at (0,0) size 0x0
selection is CARET:
start: position 3 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
upstream: position 3 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
-downstream: position 0 of child 2 {BR} of child 2 {SPAN} of root {DIV}
+downstream: position 1 of child 3 {TEXT} of root {DIV}
+2004-12-06 Ken Kocienda <kocienda@apple.com>
+
+ Reviewed by Maciej
+
+ Fix for this bug:
+
+ <rdar://problem/3890955> 8A314: Forward delete sometimes fails to delete the selected quoted text
+
+ * khtml/editing/htmlediting.cpp:
+ (khtml::DeleteSelectionCommand::handleSpecialCaseBRDelete): Fixed bonehead coding mistake in the
+ check for one of the special cases being checked for in this function. The specific case
+ intends to check for a selection that is only a <br> after a block ends (as in </div><br>). If it
+ sees such markup, it deletes only the <br> and bails. However, this code would run in *any*
+ case where a selection ended in a <br> after a block and would not delete any part of the
+ selection preceding the <br>. Bad. I have tightened the check to see that only a <br> is
+ selected.
+
+ Fixing the bug above was accomplished with an additional call to DOM::Position::downstream. This
+ new use of the function exposed this bug:
+
+ <rdar://problem/3907666> Incorrectly coded loop in Position::downstream can lead to infinite loop
+
+ * khtml/xml/dom_position.cpp:
+ (DOM::Position::downstream): I am ashamed of my first cut at this. Rewrote the loop so it does
+ not have this fatal flaw. It is a much better design as well.
+
+ * layout-tests/editing/deleting/delete-3800834-fix-expected.txt: Changes made this test
+ have what I consider to be a better result. Going with it.
+
2004-12-06 Chris Blumenberg <cblu@apple.com>
Fixed: <rdar://problem/3871718> REGRESSION (125-168): text marked bold with font that does not have bold variant copies as non-bold
// 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();
- bool startIsBRAfterBlock = downstreamEndIsBR && m_downstreamEnd.node()->enclosingBlockFlowElement() != upstreamFromBR.node()->enclosingBlockFlowElement();
+ 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;
if (currentNode != startNode && renderer->isBlockFlow()) {
if (it.current().offset() == 0) {
- NodeImpl *node = currentNode;
- while (NodeImpl *firstChild = node->firstChild()) {
- if (node->renderer()->style()->visibility() == VISIBLE && node->renderer()->isBlockFlow())
- node = firstChild;
+ // If no first child, or first visible child is a not a block, return; otherwise continue.
+ if (!currentNode->firstChild())
+ return Position(currentNode, 0);
+ for (NodeImpl *child = currentNode->firstChild(); child; child = child->nextSibling()) {
+ RenderObject *r = child->renderer();
+ if (r && r->style()->visibility() == VISIBLE) {
+ if (r->isBlockFlow())
+ break; // break causes continue code below to run.
+ else
+ return Position(child, 0);
+ }
}
- return Position(node, 0);
- }
- else
continue;
+ }
}
if (renderer->isReplaced() || renderer->isBR()) {
git://git.webkit.org / WebKit-https.git / commitdiff