+2006-10-24 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by darin
+
+ <http://bugs.webkit.org/show_bug.cgi?id=10993>
+ GMail Editor: Caret doesn't always position itself after list marker
+
+ List creation uses moveParagraphs to push content into list items.
+ moveParagraphs uses a TextIterator to restore selections after moves.
+ Some characters emitted by the TextIterator had bad ranges associated
+ with them. rangeFromLocationAndLength would skip past the range it
+ should have used when asked for ranges of length 0.
+
+ * editing/TextIterator.cpp:
+ (WebCore::TextIterator::TextIterator): No longer need to initialize a
+ removed member variable.
+ (WebCore::TextIterator::advance): An extra newline is emitted when leaving
+ some blocks. Use the same range for this newline as for the first newline.
+ We should remove this code and just emit two '\n's.
+ (WebCore::TextIterator::handleTextNode): Setup m_range.
+ (WebCore::TextIterator::handleTextBox): Ditto.
+ (WebCore::TextIterator::handleReplacedElement): Ditto.
+ (WebCore::TextIterator::handleNonTextNode): Ditto.
+ (WebCore::TextIterator::exitNode): Use an m_range from the last VisiblePosition
+ in the block we're leaving to that VP after that one.
+ (WebCore::TextIterator::emitCharacter): This function now takes in the start
+ and the end of the range associated with the emited character, and sets up m_range.
+ (WebCore::TextIterator::range): Return m_range. If it is null (we are atEnd),
+ return the end of the range used to create the iterator, as a convenience to
+ callers that use call range() on an iterator that is atEnd.
+ (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
+ Same as the changes made to TextIterator's constructor.
+ (WebCore::SimplifiedBackwardsTextIterator::advance): Use a null m_range instead of
+ a null m_positionNode to signal that we're finished.
+ (WebCore::SimplifiedBackwardsTextIterator::handleTextNode): Ditto.
+ (WebCore::SimplifiedBackwardsTextIterator::handleReplacedElement): Similar to changes
+ made to TextIterator.
+ (WebCore::SimplifiedBackwardsTextIterator::emitCharacter): Ditto.
+ (WebCore::SimplifiedBackwardsTextIterator::emitNewline): Simplified.
+ (WebCore::SimplifiedBackwardsTextIterator::range): Similar to the changes made to
+ TextIterator::range.
+ (WebCore::CharacterIterator::range): This function assumed that an iterator's
+ range() was safe to modify.
+ (WebCore::TextIterator::rangeFromLocationAndLength):
+ If the range we're looking for starts in the current chunk, this function assumed
+ that if the chunk started in a text node, it would end in the same text node. This
+ is no longer the case.
+ If the range we're looking for starts in the middle of the current chunk, I assume
+ that the chunk is inside a text node, because those are the only chunks with length
+ greater than one at the moment.
+ If the range we're looking for is a zero length range that starts/ends at the end of the
+ current chunk, we used to return the start of the next chunk, but that's wrong and
+ is what caused this bug.
+ * editing/TextIterator.h:
+ (WebCore::TextIterator::atEnd): The iterator is atEnd when m_range is null.
+ (WebCore::SimplifiedBackwardsTextIterator::atEnd):
+ * editing/visible_units.cpp:
+ (WebCore::previousBoundary): Cleaned up by using a convenience function.
+ (WebCore::nextBoundary): Ditto.
+
2006-10-24 Anders Carlsson <acarlsson@apple.com>
Reviewed by Maciej.
CircularSearchBuffer &operator=(const CircularSearchBuffer&);
};
-TextIterator::TextIterator() : m_endContainer(0), m_endOffset(0), m_positionNode(0), m_lastCharacter(0)
+TextIterator::TextIterator() : m_endContainer(0), m_endOffset(0), m_lastCharacter(0)
{
}
-TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0), m_endOffset(0), m_positionNode(0)
+TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0), m_endOffset(0)
{
if (!r)
return;
m_lastCharacter = '\n';
#ifndef NDEBUG
- // need this just because of the assert in advance()
- m_positionNode = m_node;
+ // Need this just because of the assert in advance().
+ m_range = new Range(m_node->document(), Position(m_node, 0), Position(m_node, 0));
#endif
// identify the first run
void TextIterator::advance()
{
// reset the run information
- m_positionNode = 0;
+ m_range = 0;
m_textLength = 0;
// handle remembered node that needed a newline after the text node's newline
if (m_needAnotherNewline) {
- // emit the newline, with position a collapsed range at the end of current node.
- emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+ VisiblePosition lastInNode(Position(m_node, maxDeepOffset(m_node)));
+ VisiblePosition next = lastInNode.next();
+ Position start = lastInNode.deepEquivalent();
+ Position end = next.isNull() ? start : next.deepEquivalent();
+ emitCharacter('\n', start, end);
m_needAnotherNewline = false;
return;
}
// handle remembered text box
if (m_textBox) {
handleTextBox();
- if (m_positionNode)
+ if (m_range)
return;
}
m_handledNode = handleReplacedElement();
} else
m_handledNode = handleNonTextNode();
- if (m_positionNode)
+ if (m_range)
return;
}
while (!next && m_node->parentNode()) {
m_node = m_node->parentNode();
exitNode();
- if (m_positionNode) {
+ if (m_range) {
m_handledNode = true;
m_handledChildren = true;
return;
m_handledNode = false;
m_handledChildren = false;
- // how would this ever be?
- if (m_positionNode)
+ // FIXME: How would this ever be?
+ if (m_range)
return;
}
}
if (!renderer->style()->collapseWhiteSpace()) {
int runStart = m_offset;
if (m_lastTextNodeEndedWithCollapsedSpace) {
- emitCharacter(' ', m_node, 0, runStart, runStart);
+ emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart));
return false;
}
int strLength = str.length();
if (runStart >= runEnd)
return true;
- m_positionNode = m_node;
- m_positionOffsetBaseNode = 0;
- m_positionStartOffset = runStart;
- m_positionEndOffset = runEnd;
+ m_range = new Range(m_node->document(), Position(m_node, runStart), Position(m_node, runEnd));
+
m_textCharacters = str.characters() + runStart;
m_textLength = runEnd - runStart;
bool needSpace = m_lastTextNodeEndedWithCollapsedSpace
|| (m_textBox == firstTextBox && textBoxStart == runStart && runStart > 0);
if (needSpace && !isCollapsibleWhitespace(m_lastCharacter) && m_lastCharacter) {
- emitCharacter(' ', m_node, 0, runStart, runStart);
+ emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart));
return;
}
int textBoxEnd = textBoxStart + m_textBox->m_len;
// or a run of characters that does not include a newline.
// This effectively translates newlines to spaces without copying the text.
if (str[runStart] == '\n') {
- emitCharacter(' ', m_node, 0, runStart, runStart + 1);
+ emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart + 1));
m_offset = runStart + 1;
} else {
int subrunEnd = str.find('\n', runStart);
m_offset = subrunEnd;
- m_positionNode = m_node;
- m_positionOffsetBaseNode = 0;
- m_positionStartOffset = runStart;
- m_positionEndOffset = subrunEnd;
+ m_range = new Range(m_node->document(), Position(m_node, runStart), Position(m_node, subrunEnd));
m_textCharacters = str.characters() + runStart;
m_textLength = subrunEnd - runStart;
// If we are doing a subrun that doesn't go to the end of the text box,
// come back again to finish handling this text box; don't advance to the next one.
- if (m_positionEndOffset < textBoxEnd)
+ ExceptionCode ec;
+ if (m_range->endOffset(ec) < textBoxEnd)
return;
// Advance and return
bool TextIterator::handleReplacedElement()
{
if (m_lastTextNodeEndedWithCollapsedSpace) {
- emitCharacter(' ', m_lastTextNode->parentNode(), m_lastTextNode, 1, 1);
+ emitCharacter(' ', positionAfterNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
return false;
}
- m_positionNode = m_node->parentNode();
- m_positionOffsetBaseNode = m_node;
- m_positionStartOffset = 0;
- m_positionEndOffset = 1;
+ m_range = new Range(m_node->document(), positionBeforeNode(m_node), positionAfterNode(m_node));
m_textCharacters = 0;
m_textLength = 0;
bool TextIterator::handleNonTextNode()
{
if (shouldEmitNewlineForNode(m_node)) {
- emitCharacter('\n', m_node->parentNode(), m_node, 0, 1);
+ Position start = positionBeforeNode(m_node);
+ VisiblePosition next = VisiblePosition(start).next();
+ Position end = next.isNull() ? start : next.deepEquivalent();
+ emitCharacter('\n', start, end);
} else if (m_lastCharacter != '\n' && m_lastTextNode) {
- // only add the tab or newline if not at the start of a line
+ // Only add the tab or newline if not at the start of a line.
+ // FIXME: The ranges for these emitted characters should be the part of the
+ // document that this imaginary character would occupy.
if (shouldEmitTabBeforeNode(m_node))
- emitCharacter('\t', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter('\t', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
else if (shouldEmitNewlinesBeforeAndAfterNode(m_node))
- emitCharacter('\n', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter('\n', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
- emitCharacter(' ', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+ emitCharacter(' ', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
}
return true;
void TextIterator::exitNode()
{
+ VisiblePosition lastInNode(Position(m_node, maxDeepOffset(m_node)));
+ VisiblePosition next = lastInNode.next();
+ Position start = lastInNode.deepEquivalent();
+ Position end = next.isNull() ? start : next.deepEquivalent();
+
if (m_lastTextNode && shouldEmitNewlinesBeforeAndAfterNode(m_node)) {
// use extra newline to represent margin bottom, as needed
bool addNewline = shouldEmitExtraNewlineForNode(m_node);
if (m_lastCharacter != '\n') {
- // insert a newline with a position following this block
- emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+
+ emitCharacter('\n', start, end);
// remember whether to later add a newline for the current node
assert(!m_needAnotherNewline);
m_needAnotherNewline = addNewline;
} else if (addNewline) {
// insert a newline with a position following this block
- emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+ emitCharacter('\n', start, end);
}
} else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
- emitCharacter(' ', m_node->parentNode(), m_node, 1, 1);
+ emitCharacter(' ', start, end);
}
-void TextIterator::emitCharacter(UChar c, Node *textNode, Node *offsetBaseNode, int textStartOffset, int textEndOffset)
+void TextIterator::emitCharacter(UChar c, const Position& start, const Position& end)
{
- // remember information with which to construct the TextIterator::range()
- // NOTE: textNode is often not a text node, so the range will specify child nodes of positionNode
- m_positionNode = textNode;
- m_positionOffsetBaseNode = offsetBaseNode;
- m_positionStartOffset = textStartOffset;
- m_positionEndOffset = textEndOffset;
+ m_range = start.isNull() ? 0 : new Range(start.node()->document(), start, end);
// remember information with which to construct the TextIterator::characters() and length()
m_singleCharacterBuffer = c;
PassRefPtr<Range> TextIterator::range() const
{
- // use the current run information, if we have it
- if (m_positionNode) {
- if (m_positionOffsetBaseNode) {
- int index = m_positionOffsetBaseNode->nodeIndex();
- m_positionStartOffset += index;
- m_positionEndOffset += index;
- m_positionOffsetBaseNode = 0;
- }
- return new Range(m_positionNode->document(), m_positionNode, m_positionStartOffset, m_positionNode, m_positionEndOffset);
- }
-
- // otherwise, return the end of the overall range we were given
+ if (m_range)
+ return m_range;
+
if (m_endContainer)
return new Range(m_endContainer->document(), m_endContainer, m_endOffset, m_endContainer, m_endOffset);
-
+
return 0;
}
-SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator() : m_positionNode(0)
+SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator()
{
}
SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range *r)
{
- m_positionNode = 0;
-
if (!r)
return;
#ifndef NDEBUG
// Need this just because of the assert.
- m_positionNode = endNode;
+ m_range = new Range(endNode->document(), Position(endNode, 0), Position(endNode, 0));
#endif
m_lastTextNode = 0;
void SimplifiedBackwardsTextIterator::advance()
{
- assert(m_positionNode);
+ assert(m_range);
- m_positionNode = 0;
+ m_range = 0;
m_textLength = 0;
while (m_node) {
m_handledNode = handleReplacedElement();
} else
m_handledNode = handleNonTextNode();
- if (m_positionNode)
+ if (m_range)
return;
}
m_offset = 0;
m_handledNode = false;
- if (m_positionNode)
+ if (m_range)
return;
}
}
if (!renderer->firstTextBox() && str.length() > 0)
return true;
- m_positionEndOffset = m_offset;
-
+ Position end(m_node, m_offset);
m_offset = (m_node == m_startNode) ? m_startOffset : 0;
- m_positionNode = m_node;
- m_positionStartOffset = m_offset;
- m_textLength = m_positionEndOffset - m_positionStartOffset;
- m_textCharacters = str.characters() + m_positionStartOffset;
+ Position start(m_node, m_offset);
+ m_range = new Range(m_node->document(), start, end);
+ m_textLength = end.offset() - start.offset();
+ m_textCharacters = str.characters() + start.offset();
- m_lastCharacter = str[m_positionEndOffset - 1];
+ m_lastCharacter = str[end.offset() - 1];
return true;
}
bool SimplifiedBackwardsTextIterator::handleReplacedElement()
{
- int offset = m_node->nodeIndex();
-
- m_positionNode = m_node->parentNode();
- m_positionStartOffset = offset;
- m_positionEndOffset = offset + 1;
+ m_range = new Range(m_node->document(), positionBeforeNode(m_node), positionAfterNode(m_node));
m_textCharacters = 0;
m_textLength = 0;
handleNonTextNode();
}
-void SimplifiedBackwardsTextIterator::emitCharacter(UChar c, Node *node, int startOffset, int endOffset)
+void SimplifiedBackwardsTextIterator::emitCharacter(UChar c, const Position& start, const Position& end)
{
m_singleCharacterBuffer = c;
- m_positionNode = node;
- m_positionStartOffset = startOffset;
- m_positionEndOffset = endOffset;
+
+ m_range = start.isNull() ? 0 : new Range(start.node()->document(), start, end);
+
m_textCharacters = &m_singleCharacterBuffer;
m_textLength = 1;
m_lastCharacter = c;
void SimplifiedBackwardsTextIterator::emitNewline()
{
- int offset;
-
- if (m_lastTextNode) {
- offset = m_lastTextNode->nodeIndex();
- emitCharacter('\n', m_lastTextNode->parentNode(), offset, offset + 1);
- } else {
- offset = m_node->nodeIndex();
- emitCharacter('\n', m_node->parentNode(), offset, offset + 1);
- }
+ if (m_lastTextNode)
+ emitCharacter('\n', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
+ else
+ emitCharacter('\n', positionBeforeNode(m_node), positionAfterNode(m_node));
}
PassRefPtr<Range> SimplifiedBackwardsTextIterator::range() const
{
- if (m_positionNode)
- return new Range(m_positionNode->document(), m_positionNode, m_positionStartOffset, m_positionNode, m_positionEndOffset);
+ if (m_range)
+ return m_range;
return new Range(m_startNode->document(), m_startNode, m_startOffset, m_startNode, m_startOffset);
}
PassRefPtr<Range> CharacterIterator::range() const
{
- RefPtr<Range> r = m_textIterator.range();
+ ExceptionCode ec;
+ RefPtr<Range> r = m_textIterator.range()->cloneRange(ec);
if (!m_textIterator.atEnd()) {
if (m_textIterator.length() <= 1) {
assert(m_runOffset == 0);
if (rangeLocation >= docTextPosition && rangeLocation <= docTextPosition + len) {
startRangeFound = true;
int exception = 0;
- if (textRunRange->startContainer(exception)->isTextNode()) {
+ if (rangeLocation == docTextPosition + len)
+ resultRange->setStart(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
+ else {
int offset = rangeLocation - docTextPosition;
resultRange->setStart(textRunRange->startContainer(exception), offset + textRunRange->startOffset(exception), exception);
- } else {
- if (rangeLocation == docTextPosition)
- resultRange->setStart(textRunRange->startContainer(exception), textRunRange->startOffset(exception), exception);
- else
- resultRange->setStart(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
}
}
else
resultRange->setEnd(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
}
- if (!(rangeLength == 0 && rangeEnd == docTextPosition + len)) {
- docTextPosition += len;
- break;
- }
+
+ docTextPosition += len;
+ break;
}
docTextPosition += len;
}
git://git.webkit.org / WebKit-https.git / commitdiff
