[webkit-changes] cvs commit: WebCore/khtml/xml dom_position.cpp
David
harrison at opensource.apple.com
Fri Oct 7 17:50:31 PDT 2005
harrison 05/10/07 17:50:30
Modified: . ChangeLog
khtml/editing delete_selection_command.cpp
khtml/xml dom_position.cpp
Log:
Reviewed by Justin.
"<rdar://problem/4064017> Safari crashes at -[WebCoreBridge firstRectForDOMRange:] + 92"
* khtml/editing/delete_selection_command.cpp:
(khtml::DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent):
Do not insert placeholder if selection ends at a BR.
(khtml::DeleteSelectionCommand::handleGeneralDelete):
No need to preserve starting BR because insertPlaceholderForAncestorBlockContent already did.
* khtml/xml/dom_position.cpp:
(DOM::Position::upstream):
(DOM::Position::downstream):
Fixed to return original position instead of invisible position when no suitable position found upstream.
Revision Changes Path
1.218 +18 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.217
retrieving revision 1.218
diff -u -r1.217 -r1.218
--- ChangeLog 7 Oct 2005 18:38:45 -0000 1.217
+++ ChangeLog 8 Oct 2005 00:50:26 -0000 1.218
@@ -1,3 +1,21 @@
+2005-10-07 David Harrison <harrison at apple.com>
+
+ Reviewed by Justin.
+
+ "<rdar://problem/4064017> Safari crashes at -[WebCoreBridge firstRectForDOMRange:] + 92"
+
+ * khtml/editing/delete_selection_command.cpp:
+ (khtml::DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent):
+ Do not insert placeholder if selection ends at a BR.
+
+ (khtml::DeleteSelectionCommand::handleGeneralDelete):
+ No need to preserve starting BR because insertPlaceholderForAncestorBlockContent already did.
+
+ * khtml/xml/dom_position.cpp:
+ (DOM::Position::upstream):
+ (DOM::Position::downstream):
+ Fixed to return original position instead of invisible position when no suitable position found upstream.
+
2005-10-07 Vicki Murley <vicki at apple.com>
Reviewed by Hyatt.
1.20 +2 -7 WebCore/khtml/editing/delete_selection_command.cpp
Index: delete_selection_command.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/delete_selection_command.cpp,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- delete_selection_command.cpp 3 Oct 2005 21:12:19 -0000 1.19
+++ delete_selection_command.cpp 8 Oct 2005 00:50:29 -0000 1.20
@@ -263,7 +263,7 @@
VisiblePosition afterEnd = visibleEnd.next();
if ((!afterEnd.isNull() && !inSameBlock(afterEnd, visibleEnd) && !inSameBlock(afterEnd, visibleStart)) ||
- (m_downstreamEnd == m_selectionToDelete.end() && isEndOfParagraph(visibleEnd))) {
+ (m_downstreamEnd == m_selectionToDelete.end() && isEndOfParagraph(visibleEnd) && !m_downstreamEnd.node()->hasTagName(brTag))) {
NodeImpl *block = createDefaultParagraphElement(document());
insertNodeBefore(block, m_upstreamStart.node());
addBlockPlaceholderIfNeeded(block);
@@ -347,16 +347,11 @@
// If the entire start block is selected, and the selection does not extend to the end of the
// end of a block other than the block containing the selection start, then do not delete the
// start block, otherwise delete the start block.
- // A similar case is provided to cover selections starting in BR elements.
if (startOffset == 1 && m_startNode && m_startNode->hasTagName(brTag)) {
setStartNode(m_startNode->traverseNextNode());
startOffset = 0;
}
- if (m_startBlock != m_endBlock && startOffset == 0 && m_startNode && m_startNode->hasTagName(brTag) && endAtEndOfBlock) {
- // Don't delete the BR element
- setStartNode(m_startNode->traverseNextNode());
- }
- else if (m_startBlock != m_endBlock && isStartOfBlock(VisiblePosition(m_upstreamStart, m_selectionToDelete.startAffinity()))) {
+ if (m_startBlock != m_endBlock && isStartOfBlock(VisiblePosition(m_upstreamStart, m_selectionToDelete.startAffinity()))) {
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());
1.81 +13 -23 WebCore/khtml/xml/dom_position.cpp
Index: dom_position.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_position.cpp,v
retrieving revision 1.80
retrieving revision 1.81
diff -u -r1.80 -r1.81
--- dom_position.cpp 3 Oct 2005 21:12:53 -0000 1.80
+++ dom_position.cpp 8 Oct 2005 00:50:30 -0000 1.81
@@ -336,12 +336,12 @@
// AFAIK no one has a clear, complete definition for this method and how it is used.
// Here is what I have come to understand from re-working the code after fixing PositionIterator
// for <rdar://problem/4103339>. See also Ken's comments in the header. Fundamentally, upstream()
-// scans backward in the DOM starting at "this" to return a visible DOM position that is either in
-// a text node, or just after a replaced or BR element (btw downstream() also considers empty blocks).
-// The search stops when it would have entered into a part of the DOM with a different enclosing
-// block, including a nested one. Then this method returns the highest previous position that is
-// either in an atomic node (i.e. text) or is the end of a non-atomic node (_regardless_ of
-// visibility).
+// scans backward in the DOM starting at "this" to return the closest previous visible DOM position
+// that is either in a text node, or just after a replaced or BR element (btw downstream() also
+// considers empty blocks). The search is limited to the enclosing block. If the search would
+// otherwise have entered into a part of the DOM with a different enclosing block, including a
+// nested one, the search stops and this function returns the highest previous visible DOM position
+// that is either in an atomic node (i.e. text) or is the end of a non-atomic node.
Position Position::upstream() const
{
// start at equivalent deep position
@@ -353,7 +353,6 @@
// iterate backward from there, looking for a qualified position
NodeImpl *block = startNode->enclosingBlockFlowOrTableElement();
Position lastVisible = *this;
- Position lastStreamer = *this;
Position currentPos = start;
for (; !currentPos.atStart(); currentPos = currentPos.previous()) {
NodeImpl *currentNode = currentPos.node();
@@ -362,11 +361,7 @@
// limit traversal to block or table enclosing the original element
// NOTE: This includes not going into nested blocks
if (block != currentNode->enclosingBlockFlowOrTableElement())
- return lastStreamer;
-
- // track last streamer position (regardless of visibility)
- if (isStreamer(currentPos))
- lastStreamer = currentPos;
+ return lastVisible;
// skip position in unrendered or invisible node
RenderObject *renderer = currentNode->renderer();
@@ -418,10 +413,10 @@
// for <rdar://problem/4103339>. See also Ken's comments in the header. Fundamentally, downstream()
// scans forward in the DOM starting at "this" to return the first visible DOM position that is
// either in a text node, or just before a replaced, BR element, or empty block flow element (i.e.
-// non-text nodes with no children). The search stops when it would
-// have entered into a part of the DOM with a different enclosing block, including a nested one.
-// If the search stops, this method returns the first previous position that is either in an
-// atomic node (i.e. text) or is at offset 0 (_regardless_ of visibility).
+// non-text nodes with no children). The search is limited to the enclosing block. If the search
+// would otherwise have entered into a part of the DOM with a different enclosing block, including a
+// nested one, the search stops and this function returns the first previous position that is either
+// in an atomic node (i.e. text) or is at offset 0.
Position Position::downstream() const
{
// start at equivalent deep position
@@ -433,7 +428,6 @@
// iterate forward from there, looking for a qualified position
NodeImpl *block = startNode->enclosingBlockFlowOrTableElement();
Position lastVisible = *this;
- Position lastStreamer = *this;
Position currentPos = start;
for (; !currentPos.atEnd(); currentPos = currentPos.next()) {
NodeImpl *currentNode = currentPos.node();
@@ -445,14 +439,10 @@
break;
// limit traversal to block or table enclosing the original element
- // return the last streamer position regardless of visibility
+ // return the last visible streamer position
// NOTE: This includes not going into nested blocks
if (block != currentNode->enclosingBlockFlowOrTableElement())
- return lastStreamer;
-
- // track last streamer position (regardless of visibility)
- if (isStreamer(currentPos))
- lastStreamer = currentPos;
+ return lastVisible;
// skip position in unrendered or invisible node
RenderObject *renderer = currentNode->renderer();
More information about the webkit-changes
mailing list