Reviewed by Darin.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2007 20:28:51 +0000 (20:28 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2007 20:28:51 +0000 (20:28 +0000)
        http://bugs.webkit.org/show_bug.cgi?id=15896
        More editing cleanup

        No functionality changes.

        * dom/Node.h: Moved several editing-related methods elsewhere.
        * dom/Node.cpp: (WebCore::Node::maxCharacterOffset): Renamed from maxOffset()
        to highlight that it is a match to offsetInCharacters(), and much different from other
        offset-related methods. Added ASSERT_NOT_REACHED(), as callers are supposed to check
        offsetInCharacters() before calling this.

        * dom/CharacterData.cpp: (WebCore::CharacterData::maxCharacterOffset):
        * dom/CharacterData.h: (WebCore::CharacterData::isCharacterDataNode):
        Updated for above renamings.

        * dom/Comment.{h,cpp}: Removed an override for offsetInCharacters(), which is already present in CharacterData.

        * dom/Document.{h,cpp}: Folded updateSelection() into Frame::selectionLayoutChanged().

        * dom/Position.h:
        * dom/Position.cpp:
        (WebCore::Position::uncheckedPreviousOffset): Moved from Node::previousOffset().
        (WebCore::Position::uncheckedNextOffset): Moved from Node::NextOffset().
        (WebCore::Position::previous): Adapted to the above move.
        (WebCore::Position::next): Ditto.
        (WebCore::Position::upstream): Removed an isBR() check, since a non-BR element cannot have a BR renderer (I think),
        and BR elements are covered by editingIgnoresContent().
        (WebCore::Position::downstream): Ditto.
        (WebCore::caretMaxRenderedOffset): Moved from Node::caretMaxRenderedOffset().
        (WebCore::Position::rendersInDifferentPosition): Updated for the above moves.

        * dom/PositionIterator.h: Added a comment describing this class from the original check-in.
        * dom/PositionIterator.cpp:
        (WebCore::PositionIterator::increment): Updated for the above moves.
        (WebCore::PositionIterator::decrement): Ditto.

        * dom/ProcessingInstruction.h:
        * dom/ProcessingInstruction.cpp: (WebCore::ProcessingInstruction::maxCharacterOffset):
        ProcessingInstruction was already returning true from offsetInCharacters(), but didn't override maxCharacterOffset().
        I think that implementing it has no actual effect, as PIs are not rendered, but it looks cleaner this way.

        * dom/Range.cpp:
        (WebCore::Range::selectNodeContents):
        * editing/ApplyStyleCommand.cpp:
        (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange):
        (WebCore::ApplyStyleCommand::applyInlineStyle):
        (WebCore::maxRangeOffset):
        (WebCore::ApplyStyleCommand::removeInlineStyle):
        (WebCore::ApplyStyleCommand::splitTextAtStartIfNeeded):
        (WebCore::ApplyStyleCommand::splitTextAtEndIfNeeded):
        (WebCore::ApplyStyleCommand::splitTextElementAtStartIfNeeded):
        (WebCore::ApplyStyleCommand::splitTextElementAtEndIfNeeded):
        (WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):
        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::insertNodeAt):
        (WebCore::CompositeEditCommand::positionOutsideTabSpan):
        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::handleGeneralDelete):
        * editing/InsertLineBreakCommand.cpp:
        (WebCore::InsertLineBreakCommand::doApply):
        * editing/InsertParagraphSeparatorCommand.cpp:
        (WebCore::InsertParagraphSeparatorCommand::doApply):
        * editing/InsertTextCommand.cpp:
        (WebCore::InsertTextCommand::insertTab):
        * editing/visible_units.cpp:
        (WebCore::previousLinePosition):
        (WebCore::nextLinePosition):
        Updated for the above moves.

        * editing/Editor.cpp:
        (WebCore::Editor::advanceToNextMisspelling): Added a missing rangeCompliantEquivalent() call.

        * editing/TextIterator.cpp:
        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator): Changed the condition to obviously
        match a maxCharacterOffset() call made after it; hopefully, this doesn't break any border cases.
        (WebCore::SimplifiedBackwardsTextIterator::advance): Updated for the above moves.

        * editing/htmlediting.h:
        * editing/htmlediting.cpp:
        (WebCore::canHaveChildrenForEditing): Removed a bogus comment: I don't thin BRs have a special ability to accept
        child nodes, other than via DOM manipulation, which is not specific to BRs.
        (WebCore::rangeCompliantEquivalent): Removed a check for BR, which is already covered by editingIgnoresContent().
        (WebCore::maxDeepOffset): Ditto.
        (WebCore::caretMinOffset): Moved from Node. Changed some runtime checks that seemingly cannot fail into assertions.
        (WebCore::caretMaxOffset): Ditto.

        * page/EventHandler.cpp:
        (WebCore::EventHandler::handleMousePressEventSingleClick): Pass 0 to VisiblePosition constructor instead of
        caretMinOffset. I didn't want to include htmlediting.h here, and I think that VisiblePosition constructor
        will take care of adjusting the offset.

        * page/Frame.cpp: (WebCore::Frame::selectionLayoutChanged): Folded Document::updateSelection() here.
        * page/mac/WebCoreFrameBridge.mm:
        (-[WebCoreFrameBridge smartDeleteRangeForProposedRange:]): Added missing rangeCompliantEquivalent() calls.
        * rendering/RenderBlock.cpp: (WebCore::RenderBlock::positionForRenderer): Changed to not round-trip via editing.
        Changed some runtime checks that seemingly cannot fail into assertions.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27690 268f45cc-cd09-0410-ab3c-d52691b4dbfc

32 files changed:
WebCore/ChangeLog
WebCore/dom/CharacterData.cpp
WebCore/dom/CharacterData.h
WebCore/dom/Comment.cpp
WebCore/dom/Comment.h
WebCore/dom/Document.cpp
WebCore/dom/Document.h
WebCore/dom/Node.cpp
WebCore/dom/Node.h
WebCore/dom/Position.cpp
WebCore/dom/Position.h
WebCore/dom/PositionIterator.cpp
WebCore/dom/PositionIterator.h
WebCore/dom/ProcessingInstruction.cpp
WebCore/dom/ProcessingInstruction.h
WebCore/dom/Range.cpp
WebCore/editing/ApplyStyleCommand.cpp
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/Editor.cpp
WebCore/editing/InsertLineBreakCommand.cpp
WebCore/editing/InsertParagraphSeparatorCommand.cpp
WebCore/editing/InsertTextCommand.cpp
WebCore/editing/SelectionController.cpp
WebCore/editing/TextIterator.cpp
WebCore/editing/htmlediting.cpp
WebCore/editing/htmlediting.h
WebCore/editing/visible_units.cpp
WebCore/page/EventHandler.cpp
WebCore/page/Frame.cpp
WebCore/page/mac/WebCoreFrameBridge.mm
WebCore/rendering/RenderBlock.cpp

index c13a9dce9ca0986ea8040f30e26ddf1aca659bb0..47a76f50fd7a9d862adbe452d83d5d17707c3b18 100644 (file)
@@ -1,3 +1,104 @@
+2007-11-11  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        http://bugs.webkit.org/show_bug.cgi?id=15896
+        More editing cleanup
+
+        No functionality changes.
+
+        * dom/Node.h: Moved several editing-related methods elsewhere.
+        * dom/Node.cpp: (WebCore::Node::maxCharacterOffset): Renamed from maxOffset()
+        to highlight that it is a match to offsetInCharacters(), and much different from other
+        offset-related methods. Added ASSERT_NOT_REACHED(), as callers are supposed to check
+        offsetInCharacters() before calling this.
+
+        * dom/CharacterData.cpp: (WebCore::CharacterData::maxCharacterOffset):
+        * dom/CharacterData.h: (WebCore::CharacterData::isCharacterDataNode):
+        Updated for above renamings.
+
+        * dom/Comment.{h,cpp}: Removed an override for offsetInCharacters(), which is already present in CharacterData.
+
+        * dom/Document.{h,cpp}: Folded updateSelection() into Frame::selectionLayoutChanged().
+
+        * dom/Position.h:
+        * dom/Position.cpp:
+        (WebCore::Position::uncheckedPreviousOffset): Moved from Node::previousOffset().
+        (WebCore::Position::uncheckedNextOffset): Moved from Node::NextOffset().
+        (WebCore::Position::previous): Adapted to the above move.
+        (WebCore::Position::next): Ditto.
+        (WebCore::Position::upstream): Removed an isBR() check, since a non-BR element cannot have a BR renderer (I think),
+        and BR elements are covered by editingIgnoresContent().
+        (WebCore::Position::downstream): Ditto.
+        (WebCore::caretMaxRenderedOffset): Moved from Node::caretMaxRenderedOffset().
+        (WebCore::Position::rendersInDifferentPosition): Updated for the above moves.
+
+        * dom/PositionIterator.h: Added a comment describing this class from the original check-in.
+        * dom/PositionIterator.cpp:
+        (WebCore::PositionIterator::increment): Updated for the above moves.
+        (WebCore::PositionIterator::decrement): Ditto.
+
+        * dom/ProcessingInstruction.h:
+        * dom/ProcessingInstruction.cpp: (WebCore::ProcessingInstruction::maxCharacterOffset):
+        ProcessingInstruction was already returning true from offsetInCharacters(), but didn't override maxCharacterOffset().
+        I think that implementing it has no actual effect, as PIs are not rendered, but it looks cleaner this way.
+
+        * dom/Range.cpp:
+        (WebCore::Range::selectNodeContents):
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange):
+        (WebCore::ApplyStyleCommand::applyInlineStyle):
+        (WebCore::maxRangeOffset):
+        (WebCore::ApplyStyleCommand::removeInlineStyle):
+        (WebCore::ApplyStyleCommand::splitTextAtStartIfNeeded):
+        (WebCore::ApplyStyleCommand::splitTextAtEndIfNeeded):
+        (WebCore::ApplyStyleCommand::splitTextElementAtStartIfNeeded):
+        (WebCore::ApplyStyleCommand::splitTextElementAtEndIfNeeded):
+        (WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::insertNodeAt):
+        (WebCore::CompositeEditCommand::positionOutsideTabSpan):
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleGeneralDelete):
+        * editing/InsertLineBreakCommand.cpp:
+        (WebCore::InsertLineBreakCommand::doApply):
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply):
+        * editing/InsertTextCommand.cpp:
+        (WebCore::InsertTextCommand::insertTab):
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition):
+        (WebCore::nextLinePosition):
+        Updated for the above moves.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::advanceToNextMisspelling): Added a missing rangeCompliantEquivalent() call.
+
+        * editing/TextIterator.cpp:
+        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator): Changed the condition to obviously
+        match a maxCharacterOffset() call made after it; hopefully, this doesn't break any border cases.
+        (WebCore::SimplifiedBackwardsTextIterator::advance): Updated for the above moves.
+
+        * editing/htmlediting.h:
+        * editing/htmlediting.cpp:
+        (WebCore::canHaveChildrenForEditing): Removed a bogus comment: I don't thin BRs have a special ability to accept
+        child nodes, other than via DOM manipulation, which is not specific to BRs.
+        (WebCore::rangeCompliantEquivalent): Removed a check for BR, which is already covered by editingIgnoresContent().
+        (WebCore::maxDeepOffset): Ditto.
+        (WebCore::caretMinOffset): Moved from Node. Changed some runtime checks that seemingly cannot fail into assertions.
+        (WebCore::caretMaxOffset): Ditto.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleMousePressEventSingleClick): Pass 0 to VisiblePosition constructor instead of
+        caretMinOffset. I didn't want to include htmlediting.h here, and I think that VisiblePosition constructor
+        will take care of adjusting the offset.
+
+        * page/Frame.cpp: (WebCore::Frame::selectionLayoutChanged): Folded Document::updateSelection() here.
+        * page/mac/WebCoreFrameBridge.mm:
+        (-[WebCoreFrameBridge smartDeleteRangeForProposedRange:]): Added missing rangeCompliantEquivalent() calls.
+        * rendering/RenderBlock.cpp: (WebCore::RenderBlock::positionForRenderer): Changed to not round-trip via editing.
+        Changed some runtime checks that seemingly cannot fail into assertions.
+
 2007-11-11  Darin Adler  <darin@apple.com>
 
         Reviewed by Sam.
index dc43e45187c8e7e6e77c3b1f5f2ab508edb7db13..16663963b94151fb1b47a4a8f22a3b3565a6aab9 100644 (file)
@@ -259,29 +259,11 @@ void CharacterData::checkCharDataOperation( const unsigned offset, ExceptionCode
     }
 }
 
-int CharacterData::maxOffset() const 
+int CharacterData::maxCharacterOffset() const 
 {
     return (int)length();
 }
 
-int CharacterData::caretMinOffset() const 
-{
-    RenderText *r = static_cast<RenderText *>(renderer());
-    return r && r->isText() ? r->caretMinOffset() : 0;
-}
-
-int CharacterData::caretMaxOffset() const 
-{
-    RenderText *r = static_cast<RenderText *>(renderer());
-    return r && r->isText() ? r->caretMaxOffset() : (int)length();
-}
-
-unsigned CharacterData::caretMaxRenderedOffset() const 
-{
-    RenderText *r = static_cast<RenderText *>(renderer());
-    return r ? r->caretMaxRenderedOffset() : length();
-}
-
 bool CharacterData::rendererIsNeeded(RenderStyle *style)
 {
     if (!str || str->length() == 0)
index 8c35f790166d6c3a72534a365e3633305ff56b1b..68bff438fb8509ae9e7a9b32476a2ab6876fb06d 100644 (file)
@@ -53,17 +53,14 @@ public:
 
     virtual String nodeValue() const;
     virtual void setNodeValue(const String&, ExceptionCode&);
-    virtual bool isCharacterDataNode() const { return true; }
     
     // Other methods (not part of DOM)
 
+    virtual bool isCharacterDataNode() const { return true; }
+    virtual int maxCharacterOffset() const;
     StringImpl* string() { return str; }
     virtual void checkCharDataOperation(unsigned offset, ExceptionCode&);
 
-    virtual int maxOffset() const;
-    virtual int caretMinOffset() const;
-    virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
     virtual bool offsetInCharacters() const;
     virtual bool rendererIsNeeded(RenderStyle*);
     
index fd5220f1e8b8685598c1f1c6fd5d8715847c3b9a..d4a02a2de1e93d1c32366e91258c0809a1086315 100644 (file)
@@ -74,9 +74,4 @@ String Comment::toString() const
     return "<!--" + nodeValue() + "-->";
 }
 
-bool Comment::offsetInCharacters() const
-{
-    return true;
-}
-
 } // namespace WebCore
index c6becc12f4b1d085ab8f1d19673b93d195dd0a66..8d9cd5c8f3690b2bac6cf3e3d85ee1ebef58859f 100644 (file)
@@ -45,7 +45,6 @@ public:
     // Other methods (not part of DOM)
     virtual bool isCommentNode() const { return true; }
     virtual bool childTypeAllowed(NodeType);
-    virtual bool offsetInCharacters() const;
 
     virtual String toString() const;
 };
index a884734a78d20e59a30ea62d36f5f4031efe07fc..de8d4e84da85c4a89ed15879e421800d65fb02fc 100644 (file)
@@ -1297,38 +1297,6 @@ void Document::setVisuallyOrdered()
         renderer()->style()->setVisuallyOrdered(true);
 }
 
-void Document::updateSelection()
-{
-    if (!renderer())
-        return;
-    
-    RenderView *canvas = static_cast<RenderView*>(renderer());
-    Selection selection = frame()->selectionController()->selection();
-        
-    if (!selection.isRange())
-        canvas->clearSelection();
-    else {
-        // Use the rightmost candidate for the start of the selection, and the leftmost candidate for the end of the selection.
-        // Example: foo <a>bar</a>.  Imagine that a line wrap occurs after 'foo', and that 'bar' is selected.   If we pass [foo, 3]
-        // as the start of the selection, the selection painting code will think that content on the line containing 'foo' is selected
-        // and will fill the gap before 'bar'.
-        Position startPos = selection.visibleStart().deepEquivalent();
-        if (startPos.downstream().isCandidate())
-            startPos = startPos.downstream();
-        Position endPos = selection.visibleEnd().deepEquivalent();
-        if (endPos.upstream().isCandidate())
-            endPos = endPos.upstream();
-        
-        // We can get into a state where the selection endpoints map to the same VisiblePosition when a selection is deleted
-        // because we don't yet notify the SelectionController of text removal.
-        if (startPos.isNotNull() && endPos.isNotNull() && selection.visibleStart() != selection.visibleEnd()) {
-            RenderObject *startRenderer = startPos.node()->renderer();
-            RenderObject *endRenderer = endPos.node()->renderer();
-            static_cast<RenderView*>(renderer())->setSelection(startRenderer, startPos.offset(), endRenderer, endPos.offset());
-        }
-    }
-}
-
 Tokenizer* Document::createTokenizer()
 {
     // FIXME: this should probably pass the frame instead
index 87dc25e027c3a4a391cdd625a249727c0e057b76..3ace9fe0027ebac1d6c1c0bf6a30329aa3ed82ef 100644 (file)
@@ -347,8 +347,6 @@ public:
     // to get visually ordered hebrew and arabic pages right
     void setVisuallyOrdered();
 
-    void updateSelection();
-    
     void open();
     void implicitOpen();
     void close();
index 6b5da63f64951b5b9b8f37e6f84b2cfa7ec1a4de..3f798559b8268a2192c23215b26396f64b1925b7 100644 (file)
@@ -1067,38 +1067,14 @@ RenderStyle* Node::computedStyle()
     return parent() ? parent()->computedStyle() : 0;
 }
 
-int Node::maxOffset() const
+int Node::maxCharacterOffset() const
 {
-    return 1;
+    ASSERT_NOT_REACHED();
+    return 0;
 }
 
 // FIXME: Shouldn't these functions be in the editing code?  Code that asks questions about HTML in the core DOM class
 // is obviously misplaced.
-int Node::caretMinOffset() const
-{
-    return renderer() ? renderer()->caretMinOffset() : 0;
-}
-
-int Node::caretMaxOffset() const
-{
-    return renderer() ? renderer()->caretMaxOffset() : 1;
-}
-
-unsigned Node::caretMaxRenderedOffset() const
-{
-    return renderer() ? renderer()->caretMaxRenderedOffset() : 1;
-}
-
-int Node::previousOffset (int current) const
-{
-    return renderer() ? renderer()->previousOffset(current) : current - 1;
-}
-
-int Node::nextOffset (int current) const
-{
-    return renderer() ? renderer()->nextOffset(current) : current + 1;
-}
-
 bool Node::canStartSelection() const
 {
     if (isContentEditable())
index afc3672656ab5858f935b5313b2675e1f1872caa..6d561deb09a2fea20e87b1671d98465190458090 100644 (file)
@@ -349,14 +349,9 @@ public:
     
     // Used to determine whether range offsets use characters or node indices.
     virtual bool offsetInCharacters() const;
-
-    // FIXME: These next 7 functions are mostly editing-specific and should be moved out.
-    virtual int maxOffset() const;
-    virtual int caretMinOffset() const;
-    virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
-    virtual int previousOffset(int current) const;
-    virtual int nextOffset(int current) const;
+    // Number of DOM 16-bit units contained in node. Note that rendered text length can be different - e.g. because of
+    // css-transform:capitalize breaking up precomposed characters and ligatures.
+    virtual int maxCharacterOffset() const;
     
     // FIXME: We should try to find a better location for these methods.
     virtual bool canSelectAll() const { return false; }
index 48e179a45c549a17d08f332bf995a94c8fcdc66f..bfed910554231f6da265f0c6fb6b6e4c632469ed 100644 (file)
@@ -133,7 +133,7 @@ Position Position::previous(EUsingComposedCharacters usingComposedCharacters) co
         //      Going backward one character at a time is correct.
         //   2) The old offset was a bogus offset like (<br>, 1), and there is no child.
         //      Going from 1 to 0 is correct.
-        return Position(n, usingComposedCharacters ? n->previousOffset(o) : o - 1);
+        return Position(n, usingComposedCharacters ? uncheckedPreviousOffset(n, o) : o - 1);
     }
 
     Node *parent = n->parentNode();
@@ -163,7 +163,7 @@ Position Position::next(EUsingComposedCharacters usingComposedCharacters) const
         //      Going forward one character at a time is correct.
         //   2) The new offset is a bogus offset like (<br>, 1), and there is no child.
         //      Going from 0 to 1 is correct.
-        return Position(n, usingComposedCharacters ? n->nextOffset(o) : o + 1);
+        return Position(n, usingComposedCharacters ? uncheckedNextOffset(n, o) : o + 1);
     }
 
     Node *parent = n->parentNode();
@@ -173,6 +173,16 @@ Position Position::next(EUsingComposedCharacters usingComposedCharacters) const
     return Position(parent, n->nodeIndex() + 1);
 }
 
+int Position::uncheckedPreviousOffset(const Node* n, int current)
+{
+    return n->renderer() ? n->renderer()->previousOffset(current) : current - 1;
+}
+
+int Position::uncheckedNextOffset(const Node* n, int current)
+{
+    return n->renderer() ? n->renderer()->nextOffset(current) : current + 1;
+}
+
 bool Position::atStart() const
 {
     Node *n = node();
@@ -320,9 +330,9 @@ Position Position::upstream() const
         // return lastVisible on the next iteration, but we terminate early.
         if (currentNode == enclosingBlock(currentNode) && currentPos.atStartOfNode())
             return lastVisible;
-            
-        // Return position after brs, tables, and nodes which have content that can be ignored.
-        if (editingIgnoresContent(currentNode) || renderer->isBR() || isTableElement(currentNode)) {
+
+        // Return position after tables and nodes which have content that can be ignored.
+        if (editingIgnoresContent(currentNode) || isTableElement(currentNode)) {
             if (currentPos.atEndOfNode())
                 return Position(currentNode, maxDeepOffset(currentNode));
             continue;
@@ -392,8 +402,8 @@ Position Position::downstream() const
         if (isStreamer(currentPos))
             lastVisible = currentPos;
 
-        // Return position before brs, tables, and nodes which have content that can be ignored.
-        if (editingIgnoresContent(currentNode) || renderer->isBR() || isTableElement(currentNode)) {
+        // Return position before tables and nodes which have content that can be ignored.
+        if (editingIgnoresContent(currentNode) || isTableElement(currentNode)) {
             if (currentPos.offsetInLeafNode() <= renderer->caretMinOffset())
                 return Position(currentNode, renderer->caretMinOffset());
             continue;
@@ -493,6 +503,17 @@ bool Position::inRenderedText() const
     return false;
 }
 
+static unsigned caretMaxRenderedOffset(const Node* n)
+{
+    RenderObject* r = n->renderer();
+    if (r)
+        return r->caretMaxRenderedOffset();
+    
+    if (n->isCharacterDataNode())
+        return static_cast<const CharacterData*>(n)->length();
+    return 1;
+}
+
 bool Position::isRenderedCharacter() const
 {
     if (isNull() || !node()->isTextNode())
@@ -572,8 +593,8 @@ bool Position::rendersInDifferentPosition(const Position &pos) const
     LOG(Editing, "thisRenderedOffset:         %d\n", thisRenderedOffset);
     LOG(Editing, "posRenderer:            %p [%p]\n", posRenderer, posRenderer ? posRenderer->inlineBox(offset()) : 0);
     LOG(Editing, "posRenderedOffset:      %d\n", posRenderedOffset);
-    LOG(Editing, "node min/max:           %d:%d\n", node()->caretMinOffset(), node()->caretMaxRenderedOffset());
-    LOG(Editing, "pos node min/max:       %d:%d\n", pos.node()->caretMinOffset(), pos.node()->caretMaxRenderedOffset());
+    LOG(Editing, "node min/max:           %d:%d\n", caretMinOffset(node()), caretMaxRenderedOffset(node()));
+    LOG(Editing, "pos node min/max:       %d:%d\n", caretMinOffset(pos.node()), caretMaxRenderedOffset(pos.node()));
     LOG(Editing, "----------------------------------------------------------------------\n");
 
     InlineBox *b1 = renderer ? renderer->inlineBox(offset()) : 0;
@@ -588,19 +609,19 @@ bool Position::rendersInDifferentPosition(const Position &pos) const
     }
 
     if (nextRenderedEditable(node()) == pos.node() && 
-        thisRenderedOffset == (int)node()->caretMaxRenderedOffset() && posRenderedOffset == 0) {
+        thisRenderedOffset == (int)caretMaxRenderedOffset(node()) && posRenderedOffset == 0) {
         return false;
     }
     
     if (previousRenderedEditable(node()) == pos.node() && 
-        thisRenderedOffset == 0 && posRenderedOffset == (int)pos.node()->caretMaxRenderedOffset()) {
+        thisRenderedOffset == 0 && posRenderedOffset == (int)caretMaxRenderedOffset(pos.node())) {
         return false;
     }
 
     return true;
 }
 
-// This is only called from DeleteSelectionCommand and assumes that it starts in editable content.
+// This assumes that it starts in editable content.
 Position Position::leadingWhitespacePosition(EAffinity affinity, bool considerNonCollapsibleWhitespace) const
 {
     ASSERT(isEditablePosition(*this));
@@ -622,7 +643,7 @@ Position Position::leadingWhitespacePosition(EAffinity affinity, bool considerNo
     return Position();
 }
 
-// This is only called from DeleteSelectionCommand and assumes that it starts in editable content.
+// This assumes that it starts in editable content.
 Position Position::trailingWhitespacePosition(EAffinity affinity, bool considerNonCollapsibleWhitespace) const
 {
     ASSERT(isEditablePosition(*this));
index acb7cbfe363bd24898198e1600193a839b05299c..8b99bc2e15c325c126aff904d29811ae86d54d7f 100644 (file)
@@ -57,12 +57,17 @@ public:
     Element* element() const;
     PassRefPtr<CSSComputedStyleDeclaration> computedStyle() const;
 
-    // Move up or down the DOM by one position
+    // Move up or down the DOM by one position.
+    // Offsets are computed using render text for nodes that have renderers - but note that even when
+    // using composed characters, the result may be inside a single user-visible character if a ligature is formed.
     Position previous(EUsingComposedCharacters usingComposedCharacters=NotUsingComposedCharacters) const;
     Position next(EUsingComposedCharacters usingComposedCharacters=NotUsingComposedCharacters) const;
+    static int uncheckedPreviousOffset(const Node*, int current);
+    static int uncheckedNextOffset(const Node*, int current);
+
     bool atStart() const;
     bool atEnd() const;
-    
+
     // FIXME: Make these non-member functions and put them somewhere in the editing directory.
     // These aren't really basic "position" operations. More high level editing helper functions.
     Position leadingWhitespacePosition(EAffinity, bool considerNonCollapsibleWhitespace = false) const;
index be1e59f3aa5979547dada449c8da9ae48d248863..110b14ed62df0d61d6094365c5bd0f578997bf4e 100644 (file)
@@ -47,7 +47,7 @@ void PositionIterator::increment()
     }
 
     if (!m_parent->hasChildNodes() && m_offset < maxDeepOffset(m_parent))
-        m_offset = m_parent->nextOffset(m_offset);
+        m_offset = Position::uncheckedNextOffset(m_parent, m_offset);
     else {
         m_child = m_parent;
         m_parent = m_child->parentNode();
@@ -75,7 +75,7 @@ void PositionIterator::decrement()
     }
 
     if (m_offset) {
-        m_offset = m_parent->previousOffset(m_offset);
+        m_offset = Position::uncheckedPreviousOffset(m_parent, m_offset);
     } else {
         if (m_parent->hasChildNodes()) {
             m_parent = m_parent->lastChild();
index f9e84608a169fddca45cc07dd396f355b22fb01e..9c0038f7afa9fc19ec0879bd8d40886e117c111b 100644 (file)
@@ -30,6 +30,9 @@
 
 namespace WebCore {
 
+// A Position iterator with constant-time
+// increment, decrement, and several predicates on the Position it is at.
+// Conversion to/from Position is O(n) in the offset.
 class PositionIterator {
 public:
     PositionIterator()
index b4d8a82e0adc30069c79feb118de0c40d5c5253c..7486e9a842d52a988d758817b776a5b937760b42 100644 (file)
@@ -248,4 +248,9 @@ bool ProcessingInstruction::offsetInCharacters() const
     return true;
 }
 
+int ProcessingInstruction::maxCharacterOffset() const 
+{
+    return static_cast<int>(m_data.length());
+}
+
 } // namespace
index 1fbd7ef8d63f0929080561aeae0edbf97e6c5376..8df21f72d785301c727b8f8d7efaf7aba64ff769 100644 (file)
@@ -51,6 +51,7 @@ public:
     virtual PassRefPtr<Node> cloneNode(bool deep);
     virtual bool childTypeAllowed(NodeType);
     virtual bool offsetInCharacters() const;
+    virtual int maxCharacterOffset() const;
 
     // Other methods (not part of DOM)
     String localHref() const { return m_localHref; }
index 2e4419e4bd0318c0d3cef2044101912493b20bf7..3fe5f71eed3bc6ce86c98eee945610bb6d9326c4 100644 (file)
@@ -1347,7 +1347,7 @@ void Range::selectNodeContents( Node *refNode, ExceptionCode& ec)
     m_startContainer = refNode;
     m_startOffset = 0;
     m_endContainer = refNode;
-    m_endOffset = refNode->offsetInCharacters() ? refNode->maxOffset() : refNode->childNodeCount();
+    m_endOffset = refNode->offsetInCharacters() ? refNode->maxCharacterOffset() : refNode->childNodeCount();
 }
 
 void Range::surroundContents(PassRefPtr<Node> passNewParent, ExceptionCode& ec)
index 6e0c11a3c8443c73ad03a4808179c0575066abf2..c107aefe8447fefac5deb6ab2da3f8e4099d972b 100644 (file)
@@ -491,7 +491,7 @@ void ApplyStyleCommand::applyRelativeFontStyleChange(CSSMutableStyleDeclaration
     
     start = start.upstream(); // Move upstream to ensure we do not add redundant spans.
     Node *startNode = start.node();
-    if (startNode->isTextNode() && start.offset() >= startNode->caretMaxOffset()) // Move out of text node if range does not include its characters.
+    if (startNode->isTextNode() && start.offset() >= caretMaxOffset(startNode)) // Move out of text node if range does not include its characters.
         startNode = startNode->traverseNextNode();
 
     // Store away font size before making any changes to the document.
@@ -643,7 +643,7 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclaration *style)
     
     bool rangeIsEmpty = false;
 
-    if (start.offset() >= start.node()->caretMaxOffset()) {
+    if (start.offset() >= caretMaxOffset(start.node())) {
         node = node->traverseNextNode();
         Position newStart = Position(node, 0);
         if (Range::compareBoundaryPoints(end, newStart) < 0)
@@ -949,7 +949,7 @@ void ApplyStyleCommand::pushDownTextDecorationStyleAtBoundaries(const Position &
 static int maxRangeOffset(Node *n)
 {
     if (n->offsetInCharacters())
-        return n->maxOffset();
+        return n->maxCharacterOffset();
 
     if (n->isElementNode())
         return n->childNodeCount();
@@ -998,7 +998,7 @@ void ApplyStyleCommand::removeInlineStyle(PassRefPtr<CSSMutableStyleDeclaration>
                 if (s.node() == elem) {
                     // Since elem must have been fully selected, and it is at the start
                     // of the selection, it is clear we can set the new s offset to 0.
-                    ASSERT(s.offset() <= s.node()->caretMinOffset());
+                    ASSERT(s.offset() <= caretMinOffset(s.node()));
                     s = Position(next, 0);
                 }
                 if (e.node() == elem) {
@@ -1045,7 +1045,7 @@ bool ApplyStyleCommand::nodeFullyUnselected(Node *node, const Position &start, c
 
 bool ApplyStyleCommand::splitTextAtStartIfNeeded(const Position &start, const Position &end)
 {
-    if (start.node()->isTextNode() && start.offset() > start.node()->caretMinOffset() && start.offset() < start.node()->caretMaxOffset()) {
+    if (start.node()->isTextNode() && start.offset() > caretMinOffset(start.node()) && start.offset() < caretMaxOffset(start.node())) {
         int endOffsetAdjustment = start.node() == end.node() ? start.offset() : 0;
         Text *text = static_cast<Text *>(start.node());
         splitTextNode(text, start.offset());
@@ -1057,7 +1057,7 @@ bool ApplyStyleCommand::splitTextAtStartIfNeeded(const Position &start, const Po
 
 bool ApplyStyleCommand::splitTextAtEndIfNeeded(const Position &start, const Position &end)
 {
-    if (end.node()->isTextNode() && end.offset() > end.node()->caretMinOffset() && end.offset() < end.node()->caretMaxOffset()) {
+    if (end.node()->isTextNode() && end.offset() > caretMinOffset(end.node()) && end.offset() < caretMaxOffset(end.node())) {
         Text *text = static_cast<Text *>(end.node());
         splitTextNode(text, end.offset());
         
@@ -1065,7 +1065,7 @@ bool ApplyStyleCommand::splitTextAtEndIfNeeded(const Position &start, const Posi
         ASSERT(prevNode);
         Node *startNode = start.node() == end.node() ? prevNode : start.node();
         ASSERT(startNode);
-        updateStartEnd(Position(startNode, start.offset()), Position(prevNode, prevNode->caretMaxOffset()));
+        updateStartEnd(Position(startNode, start.offset()), Position(prevNode, caretMaxOffset(prevNode)));
         return true;
     }
     return false;
@@ -1073,7 +1073,7 @@ bool ApplyStyleCommand::splitTextAtEndIfNeeded(const Position &start, const Posi
 
 bool ApplyStyleCommand::splitTextElementAtStartIfNeeded(const Position &start, const Position &end)
 {
-    if (start.node()->isTextNode() && start.offset() > start.node()->caretMinOffset() && start.offset() < start.node()->caretMaxOffset()) {
+    if (start.node()->isTextNode() && start.offset() > caretMinOffset(start.node()) && start.offset() < caretMaxOffset(start.node())) {
         int endOffsetAdjustment = start.node() == end.node() ? start.offset() : 0;
         Text *text = static_cast<Text *>(start.node());
         splitTextNodeContainingElement(text, start.offset());
@@ -1086,7 +1086,7 @@ bool ApplyStyleCommand::splitTextElementAtStartIfNeeded(const Position &start, c
 
 bool ApplyStyleCommand::splitTextElementAtEndIfNeeded(const Position &start, const Position &end)
 {
-    if (end.node()->isTextNode() && end.offset() > end.node()->caretMinOffset() && end.offset() < end.node()->caretMaxOffset()) {
+    if (end.node()->isTextNode() && end.offset() > caretMinOffset(end.node()) && end.offset() < caretMaxOffset(end.node())) {
         Text *text = static_cast<Text *>(end.node());
         splitTextNodeContainingElement(text, end.offset());
 
@@ -1183,7 +1183,7 @@ bool ApplyStyleCommand::mergeEndWithNextIfIdentical(const Position &start, const
     int endOffset = end.offset();
 
     if (isAtomicNode(endNode)) {
-        if (endOffset < endNode->caretMaxOffset())
+        if (endOffset < caretMaxOffset(endNode))
             return false;
 
         unsigned parentLastOffset = end.node()->parent()->childNodes()->length() - 1;
index 1ab36a63dee9b6cda5260fed5083e1ba5464877f..8bbe1622250b287f3a3423818264621c7258b07a 100644 (file)
@@ -156,9 +156,9 @@ void CompositeEditCommand::insertNodeAt(Node* insertChild, const Position& editi
             insertNodeBefore(insertChild, child);
         else
             appendNode(insertChild, refChild);
-    } else if (refChild->caretMinOffset() >= offset) {
+    } else if (caretMinOffset(refChild) >= offset) {
         insertNodeBefore(insertChild, refChild);
-    } else if (refChild->isTextNode() && refChild->caretMaxOffset() > offset) {
+    } else if (refChild->isTextNode() && caretMaxOffset(refChild) > offset) {
         splitTextNode(static_cast<Text *>(refChild), offset);
         insertNodeBefore(insertChild, refChild);
     } else {
@@ -290,10 +290,10 @@ Position CompositeEditCommand::positionOutsideTabSpan(const Position& pos)
     
     Node* tabSpan = tabSpanNode(pos.node());
     
-    if (pos.offset() <= pos.node()->caretMinOffset())
+    if (pos.offset() <= caretMinOffset(pos.node()))
         return positionBeforeNode(tabSpan);
         
-    if (pos.offset() >= pos.node()->caretMaxOffset())
+    if (pos.offset() >= caretMaxOffset(pos.node()))
         return positionAfterNode(tabSpan);
 
     splitTextNodeContainingElement(static_cast<Text *>(pos.node()), pos.offset());
index 1a6771a241f0675baaa463639d241dcd2977bedc..e2fe7d6a6d2c0b5175d243c2514c3c4aa6bff182 100644 (file)
@@ -371,10 +371,10 @@ void DeleteSelectionCommand::handleGeneralDelete()
         startNode = startNode->traverseNextNode();
     }
 
-    if (startOffset >= startNode->caretMaxOffset() && startNode->isTextNode()) {
+    if (startOffset >= caretMaxOffset(startNode) && startNode->isTextNode()) {
         Text *text = static_cast<Text *>(startNode);
-        if (text->length() > (unsigned)startNode->caretMaxOffset())
-            deleteTextFromNode(text, startNode->caretMaxOffset(), text->length() - startNode->caretMaxOffset());
+        if (text->length() > (unsigned)caretMaxOffset(startNode))
+            deleteTextFromNode(text, caretMaxOffset(startNode), text->length() - caretMaxOffset(startNode));
     }
 
     if (startOffset >= maxDeepOffset(startNode)) {
@@ -436,7 +436,7 @@ void DeleteSelectionCommand::handleGeneralDelete()
                 node = nextNode.get();
             } else {
                 Node* n = node->lastDescendant();
-                if (m_downstreamEnd.node() == n && m_downstreamEnd.offset() >= n->caretMaxOffset()) {
+                if (m_downstreamEnd.node() == n && m_downstreamEnd.offset() >= caretMaxOffset(n)) {
                     removeNode(node.get());
                     node = 0;
                 } else
@@ -444,7 +444,7 @@ void DeleteSelectionCommand::handleGeneralDelete()
             }
         }
         
-        if (m_downstreamEnd.node() != startNode && !m_upstreamStart.node()->isDescendantOf(m_downstreamEnd.node()) && m_downstreamEnd.node()->inDocument() && m_downstreamEnd.offset() >= m_downstreamEnd.node()->caretMinOffset()) {
+        if (m_downstreamEnd.node() != startNode && !m_upstreamStart.node()->isDescendantOf(m_downstreamEnd.node()) && m_downstreamEnd.node()->inDocument() && m_downstreamEnd.offset() >= caretMinOffset(m_downstreamEnd.node())) {
             if (m_downstreamEnd.offset() >= maxDeepOffset(m_downstreamEnd.node()) && !canHaveChildrenForEditing(m_downstreamEnd.node())) {
                 // FIXME: Shouldn't remove m_downstreamEnd.node() if its offsets refer to children. 
                 // The node itself is fully selected, not just its contents.  Delete it.
index c996ec33c85bb1a30b98f70af81a059f4b278521..e37bce7438b112d851bf4f483c7c892567a142bc 100644 (file)
@@ -2018,7 +2018,8 @@ void Editor::advanceToNextMisspelling(bool startBeforeSelection)
         if (position.isNull())
             return;
         
-        spellingSearchRange->setStart(position.node(), position.offset(), ec);
+        Position rangeCompliantPosition = rangeCompliantEquivalent(position);
+        spellingSearchRange->setStart(rangeCompliantPosition.node(), rangeCompliantPosition.offset(), ec);
         startedWithSelection = false;   // won't need to wrap
     }
     
index 9d40dc13dd9c75eb52da3aedf45acbe4e16716dc..cd1067b4a5f9b3bd873c56cb834e86114442b3ba 100644 (file)
@@ -115,7 +115,7 @@ void InsertLineBreakCommand::doApply()
         
         VisiblePosition endingPosition(Position(nodeToInsert.get(), 0));
         setEndingSelection(Selection(endingPosition));
-    } else if (pos.offset() <= pos.node()->caretMinOffset()) {
+    } else if (pos.offset() <= caretMinOffset(pos.node())) {
         insertNodeAt(nodeToInsert.get(), pos);
         
         // Insert an extra br or '\n' if the just inserted one collapsed.
@@ -123,7 +123,7 @@ void InsertLineBreakCommand::doApply()
             insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
         
         setEndingSelection(Selection(positionAfterNode(nodeToInsert.get()), DOWNSTREAM));
-    } else if (pos.offset() >= pos.node()->caretMaxOffset()) {
+    } else if (pos.offset() >= caretMaxOffset(pos.node())) {
         insertNodeAt(nodeToInsert.get(), pos);
         setEndingSelection(Selection(positionAfterNode(nodeToInsert.get()), DOWNSTREAM));
     } else {
index bea04e796d768acac5afd905fab4aaf9afe10cd2..254314909ade202ee6d49bac9a41f0e408293094 100644 (file)
@@ -260,7 +260,7 @@ void InsertParagraphSeparatorCommand::doApply()
     // Move the start node and the siblings of the start node.
     if (startNode != startBlock) {
         Node *n = startNode;
-        if (pos.offset() >= startNode->caretMaxOffset())
+        if (pos.offset() >= caretMaxOffset(startNode))
             n = startNode->nextSibling();
 
         while (n && n != blockToInsert) {
index 5700adc930fa0a2e620195483a3bac66e1852acd..5de48d96d5ec793155396a295905833f90ccd57d 100644 (file)
@@ -192,7 +192,7 @@ Position InsertTextCommand::insertTab(const Position& pos)
     }
     
     // return the position following the new tab
-    return Position(spanNode->lastChild(), spanNode->lastChild()->caretMaxOffset());
+    return Position(spanNode->lastChild(), caretMaxOffset(spanNode->lastChild()));
 }
 
 bool InsertTextCommand::isInsertTextCommand() const
index b0115d429e16968fd9b5a4dd27aecbf9bd6965d3..b71b91173ec90cb742f089e2c411c51f9f2c4d39 100644 (file)
@@ -931,7 +931,7 @@ void SelectionController::extend(Node* node, int offset, ExceptionCode& ec)
         return;
     }
     if (offset < 0
-        || node->offsetInCharacters() && offset > node->caretMaxOffset()
+        || node->offsetInCharacters() && offset > caretMaxOffset(node)
         || !node->offsetInCharacters() && offset > (int)node->childNodeCount()) {
         ec = INDEX_SIZE_ERR;
         return;
index aca8caeff8bdaa553a40c75459ade6c25f9accbe..2b827b0ca4775db9df03ee80f89ab613f29b3d53 100644 (file)
@@ -708,7 +708,7 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range *r)
     if (!endNode->offsetInCharacters()) {
         if (endOffset > 0 && endOffset <= static_cast<int>(endNode->childNodeCount())) {
             endNode = endNode->childNode(endOffset - 1);
-            endOffset = endNode->hasChildNodes() ? endNode->childNodeCount() : endNode->maxOffset();
+            endOffset = endNode->offsetInCharacters() ? endNode->maxCharacterOffset() : endNode->childNodeCount();
         }
     }
 
@@ -798,7 +798,7 @@ void SimplifiedBackwardsTextIterator::advance()
         }
         
         m_node = next;
-        m_offset = m_node ? m_node->caretMaxOffset() : 0;
+        m_offset = m_node ? caretMaxOffset(m_node) : 0;
         m_handledNode = false;
         m_handledChildren = false;
         
index d7aadd035818ea6100ef3894d421cdb3a7bcab64..d2636d0b50eb90af21b86c1f9af503750e8430d3 100644 (file)
@@ -61,7 +61,6 @@ bool editingIgnoresContent(const Node* node)
     return !canHaveChildrenForEditing(node) && !node->isTextNode();
 }
 
-// Some nodes, like brs, will technically accept children, but we don't want that to happen while editing. 
 bool canHaveChildrenForEditing(const Node* node)
 {
     return !node->hasTagName(hrTag) &&
@@ -311,13 +310,13 @@ Position rangeCompliantEquivalent(const Position& pos)
     Node *node = pos.node();
     
     if (pos.offset() <= 0) {
-        if (node->parentNode() && (node->hasTagName(brTag) || editingIgnoresContent(node)) || isTableElement(node))
+        if (node->parentNode() && (editingIgnoresContent(node) || isTableElement(node)))
             return positionBeforeNode(node);
         return Position(node, 0);
     }
     
     if (node->offsetInCharacters())
-        return Position(node, min(node->maxOffset(), pos.offset()));
+        return Position(node, min(node->maxCharacterOffset(), pos.offset()));
     
     int maxCompliantOffset = node->childNodeCount();
     if (pos.offset() > maxCompliantOffset) {
@@ -356,13 +355,13 @@ int maxDeepOffset(const Node *node)
     if (!node)
         return 0;
     if (node->offsetInCharacters())
-        return node->maxOffset();
+        return node->maxCharacterOffset();
         
     if (node->hasChildNodes())
         return node->childNodeCount();
     
     // NOTE: This should preempt the childNodeCount for, e.g., select nodes
-    if (node->hasTagName(brTag) || editingIgnoresContent(node))
+    if (editingIgnoresContent(node))
         return 1;
 
     return 0;
@@ -868,6 +867,27 @@ bool isMailBlockquote(const Node *node)
     return static_cast<const Element *>(node)->getAttribute("type") == "cite";
 }
 
+int caretMinOffset(const Node* n)
+{
+    RenderObject* r = n->renderer();
+    ASSERT(!n->isCharacterDataNode() || !r || r->isText()); // FIXME: This was a runtime check that seemingly couldn't fail; changed it to an assertion for now.
+    return r ? r->caretMinOffset() : 0;
+}
+
+int caretMaxOffset(const Node* n)
+{
+    RenderObject* r = n->renderer();
+    ASSERT(!n->isCharacterDataNode() || !r || r->isText()); // FIXME: This was a runtime check that seemingly couldn't fail; changed it to an assertion for now.
+    if (r)
+        return r->caretMaxOffset();
+
+    if (n->isCharacterDataNode()) {
+        const CharacterData* c = static_cast<const CharacterData*>(n);
+        return static_cast<int>(c->length());
+    }
+    return 1;
+}
+
 bool lineBreakExistsAtPosition(const VisiblePosition& visiblePosition)
 {
     if (visiblePosition.isNull())
index 351730d358fea1121751812a066cd295c65cb657..37e7667ca734764fced13720d417d0d54fdf5e84 100644 (file)
@@ -94,6 +94,8 @@ PassRefPtr<Element> createTabSpanElement(Document*, const String& tabText);
 bool isNodeRendered(const Node*);
 bool isMailBlockquote(const Node*);
 Node* nearestMailBlockquote(const Node*);
+int caretMinOffset(const Node*);
+int caretMaxOffset(const Node*);
 
 //------------------------------------------------------------------------------------------
 
index d4185802811f83814ccb327164172b6813e98751..7110f3eb937a41164a27c9cec852711bfd9a6d8f 100644 (file)
@@ -459,10 +459,10 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
         while (n) {
             if (highestEditableRoot(Position(n, 0)) != highestRoot)
                 break;
-            Position pos(n, n->caretMinOffset());
+            Position pos(n, caretMinOffset(n));
             if (pos.isCandidate()) {
                 ASSERT(n->renderer());
-                box = n->renderer()->inlineBox(n->caretMaxOffset());
+                box = n->renderer()->inlineBox(caretMaxOffset(n));
                 if (box) {
                     // previous root line box found
                     root = box->root();
@@ -529,10 +529,10 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
         while (n) {
             if (highestEditableRoot(Position(n, 0)) != highestRoot)
                 break;
-            Position pos(n, n->caretMinOffset());
+            Position pos(n, caretMinOffset(n));
             if (pos.isCandidate()) {
                 ASSERT(n->renderer());
-                box = n->renderer()->inlineBox(n->caretMinOffset());
+                box = n->renderer()->inlineBox(caretMinOffset(n));
                 if (box) {
                     // next root line box found
                     root = box->root();
index d6c8ce961d898111c4a9d145f23c51d19cd865f6..9da32e3da7d28a9b1af3f8a8fc5262d43015b646 100644 (file)
@@ -259,7 +259,7 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
 
     VisiblePosition visiblePos(innerNode->renderer()->positionForPoint(event.localPoint()));
     if (visiblePos.isNull())
-        visiblePos = VisiblePosition(innerNode, innerNode->caretMinOffset(), DOWNSTREAM);
+        visiblePos = VisiblePosition(innerNode, 0, DOWNSTREAM);
     Position pos = visiblePos.deepEquivalent();
     
     Selection newSelection = m_frame->selectionController()->selection();
index 7106418d8b4eb25a1d400f05d5eb58af9c344d5f..9f4cb5be2e15714bfc13eea90d66f711139cf9a5 100644 (file)
@@ -630,8 +630,34 @@ void Frame::selectionLayoutChanged()
         }
     }
 
-    if (d->m_doc)
-        d->m_doc->updateSelection();
+    if (!renderer())
+        return;
+    RenderView* canvas = static_cast<RenderView*>(renderer());
+
+    Selection selection = selectionController()->selection();
+        
+    if (!selection.isRange())
+        canvas->clearSelection();
+    else {
+        // Use the rightmost candidate for the start of the selection, and the leftmost candidate for the end of the selection.
+        // Example: foo <a>bar</a>.  Imagine that a line wrap occurs after 'foo', and that 'bar' is selected.   If we pass [foo, 3]
+        // as the start of the selection, the selection painting code will think that content on the line containing 'foo' is selected
+        // and will fill the gap before 'bar'.
+        Position startPos = selection.visibleStart().deepEquivalent();
+        if (startPos.downstream().isCandidate())
+            startPos = startPos.downstream();
+        Position endPos = selection.visibleEnd().deepEquivalent();
+        if (endPos.upstream().isCandidate())
+            endPos = endPos.upstream();
+        
+        // We can get into a state where the selection endpoints map to the same VisiblePosition when a selection is deleted
+        // because we don't yet notify the SelectionController of text removal.
+        if (startPos.isNotNull() && endPos.isNotNull() && selection.visibleStart() != selection.visibleEnd()) {
+            RenderObject *startRenderer = startPos.node()->renderer();
+            RenderObject *endRenderer = endPos.node()->renderer();
+            canvas->setSelection(startRenderer, startPos.offset(), endRenderer, endPos.offset());
+        }
+    }
 }
 
 void Frame::caretBlinkTimerFired(Timer<Frame>*)
index 5b0e3b0687e8f510e2b4ef51737ac88f971343eb..4c4ebe98708c3e817177f6c3ffb2dfeffa766645 100644 (file)
@@ -964,6 +964,9 @@ static HTMLFormElement *formElementFromDOMElement(DOMElement *element)
     if (newEnd.isNull())
         newEnd = end;
 
+    newStart = rangeCompliantEquivalent(newStart);
+    newEnd = rangeCompliantEquivalent(newEnd);
+
     RefPtr<Range> range = m_frame->document()->createRange();
     int exception = 0;
     range->setStart(newStart.node(), newStart.offset(), exception);
index 2f256de2731bf9659992a09fa7e5f5a667818d66..f608f744b2fd21d590b2670c360088a85ea18541 100644 (file)
@@ -2982,16 +2982,22 @@ Position RenderBlock::positionForBox(InlineBox *box, bool start) const
     return Position(box->object()->element(), start ? textBox->start() : textBox->start() + textBox->len());
 }
 
-Position RenderBlock::positionForRenderer(RenderObject *renderer, bool start) const
+Position RenderBlock::positionForRenderer(RenderObjectrenderer, bool start) const
 {
     if (!renderer)
         return Position(element(), 0);
 
-    Node *node = renderer->element() ? renderer->element() : element();
+    Nodenode = renderer->element() ? renderer->element() : element();
     if (!node)
         return Position();
 
-    int offset = start ? node->caretMinOffset() : node->caretMaxOffset();
+    ASSERT(renderer == node->renderer());
+
+    int offset = start ? renderer->caretMinOffset() : renderer->caretMaxOffset();
+
+    // FIXME: This was a runtime check that seemingly couldn't fail; changed it to an assertion for now.
+    ASSERT(!node->isCharacterDataNode() || renderer->isText());
+
     return Position(node, offset);
 }