Commit 17bab067 authored by harrison's avatar harrison

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@10786 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 7ff21879
2005-10-07 David Harrison <harrison@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@apple.com>
Reviewed by Hyatt.
......
......@@ -263,7 +263,7 @@ void DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent()
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 @@ void DeleteSelectionCommand::handleGeneralDelete()
// 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());
......
......@@ -336,12 +336,12 @@ static bool isStreamer (Position pos)
// 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 @@ Position Position::upstream() const
// 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 @@ Position Position::upstream() const
// 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 @@ Position Position::upstream() const
// 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 @@ Position Position::downstream() const
// 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 @@ Position Position::downstream() const
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();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment