Changeset 96187 in webkit

Timestamp:

Sep 27, 2011 10:04:20 PM (13 years ago)

Author:

rniwa@webkit.org

Message:

Simplify ReplaceSelectionCommand::positionAtStartOfInsertedContent
​https://bugs.webkit.org/show_bug.cgi?id=68939

Reviewed by Darin Adler.

Simplified ReplaceSelectionCommand::positionAtStartOfInsertedContent.

This change revealed a bug in removeUnrenderedTextNodesAtEnds that text nodes without any visible
text at ends are not removed when it has a render object. Fixed the bug by checking the length of
the rendered text. (Tested by editing/pasteboard/pasting-word-in-div-extra-line.html)

This further revealed that caretMaxRenderedOffset doesn't return an offset and caretMaxRenderedOffset
on InlineBox, InlineTextBox, RenderObject, RenderBR, RenderPlaced are never called. To address this
issue, renamed caretMaxRenderedOffset to renderedTextLength for RenderText and removed the rest.

• dom/Position.cpp:

(WebCore::Position::rendersInDifferentPosition):

• editing/ReplaceSelectionCommand.cpp:

(WebCore::nodeHasVisibleRenderText): Added.
(WebCore::ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds): Calls nodeHasVisibleRenderText.
(WebCore::ReplaceSelectionCommand::positionAtStartOfInsertedContent): Simplified.

• editing/visible_units.cpp:

(WebCore::startOfParagraph): Calls renderedTextLength.
(WebCore::endOfParagraph): Ditto.

• rendering/InlineBox.cpp: Removed caretMaxRenderedOffset.
• rendering/InlineBox.h: Ditto.
• rendering/InlineTextBox.cpp: Ditto.
• rendering/InlineTextBox.h: Ditto.
• rendering/RenderBR.cpp: Ditto.
• rendering/RenderBR.h: Ditto.
• rendering/RenderObject.cpp: Ditto.
• rendering/RenderObject.h: Ditto.
• rendering/RenderReplaced.cpp: Ditto.
• rendering/RenderReplaced.h: Ditto.
• rendering/RenderText.cpp:

(WebCore::RenderText::renderedTextLength): Renamed from caretMaxRenderedOffset.

• rendering/RenderText.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Position.cpp (modified) (3 diffs)
• editing/ReplaceSelectionCommand.cpp (modified) (4 diffs)
• editing/visible_units.cpp (modified) (2 diffs)
• rendering/InlineBox.cpp (modified) (1 diff)
• rendering/InlineBox.h (modified) (1 diff)
• rendering/InlineTextBox.cpp (modified) (1 diff)
• rendering/InlineTextBox.h (modified) (1 diff)
• rendering/RenderBR.cpp (modified) (1 diff)
• rendering/RenderBR.h (modified) (1 diff)
• rendering/RenderObject.cpp (modified) (1 diff)
• rendering/RenderObject.h (modified) (1 diff)
• rendering/RenderReplaced.cpp (modified) (1 diff)
• rendering/RenderReplaced.h (modified) (1 diff)
• rendering/RenderText.cpp (modified) (2 diffs)
• rendering/RenderText.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Simplify ReplaceSelectionCommand::positionAtStartOfInsertedContent
+        https://bugs.webkit.org/show_bug.cgi?id=68939
+
+        Reviewed by Darin Adler.
+
+        Simplified ReplaceSelectionCommand::positionAtStartOfInsertedContent.
+
+        This change revealed a bug in removeUnrenderedTextNodesAtEnds that text nodes without any visible
+        text at ends are not removed when it has a render object. Fixed the bug by checking the length of
+        the rendered text. (Tested by editing/pasteboard/pasting-word-in-div-extra-line.html)
+
+        This further revealed that caretMaxRenderedOffset doesn't return an offset and caretMaxRenderedOffset
+        on InlineBox, InlineTextBox, RenderObject, RenderBR, RenderPlaced are never called. To address this
+        issue, renamed caretMaxRenderedOffset to renderedTextLength for RenderText and removed the rest.
+
+        * dom/Position.cpp:
+        (WebCore::Position::rendersInDifferentPosition):
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::nodeHasVisibleRenderText): Added.
+        (WebCore::ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds): Calls nodeHasVisibleRenderText.
+        (WebCore::ReplaceSelectionCommand::positionAtStartOfInsertedContent): Simplified.
+        * editing/visible_units.cpp:
+        (WebCore::startOfParagraph): Calls renderedTextLength.
+        (WebCore::endOfParagraph): Ditto.
+        * rendering/InlineBox.cpp: Removed caretMaxRenderedOffset.
+        * rendering/InlineBox.h: Ditto.
+        * rendering/InlineTextBox.cpp: Ditto.
+        * rendering/InlineTextBox.h: Ditto.
+        * rendering/RenderBR.cpp: Ditto.
+        * rendering/RenderBR.h: Ditto.
+        * rendering/RenderObject.cpp: Ditto.
+        * rendering/RenderObject.h: Ditto.
+        * rendering/RenderReplaced.cpp: Ditto.
+        * rendering/RenderReplaced.h: Ditto.
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::renderedTextLength): Renamed from caretMaxRenderedOffset.
+        * rendering/RenderText.h:
+
 2011-09-27  James Robinson  <jamesr@chromium.org>
 

trunk/Source/WebCore/dom/Position.cpp

@@ -897 +897 @@
 }
 
-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
 {
@@ -993 +982 @@
     LOG(Editing, "posRenderer:            %p [%p]\n", posRenderer, b2);
     LOG(Editing, "posRenderedOffset:      %d\n", posRenderedOffset);
-    LOG(Editing, "node min/max:           %d:%d\n", caretMinOffset(deprecatedNode()), caretMaxRenderedOffset(deprecatedNode()));
-    LOG(Editing, "pos node min/max:       %d:%d\n", caretMinOffset(pos.deprecatedNode()), caretMaxRenderedOffset(pos.deprecatedNode()));
+    LOG(Editing, "node min/max:           %d:%d\n", caretMinOffset(deprecatedNode()), caretMaxOffset(deprecatedNode()));
+    LOG(Editing, "pos node min/max:       %d:%d\n", caretMinOffset(pos.deprecatedNode()), caretMaxOffset(pos.deprecatedNode()));
     LOG(Editing, "----------------------------------------------------------------------\n");
 
@@ -1006 +995 @@
 
     if (nextRenderedEditable(deprecatedNode()) == pos.deprecatedNode()
-        && thisRenderedOffset == (int)caretMaxRenderedOffset(deprecatedNode()) && !posRenderedOffset) {
+        && thisRenderedOffset == caretMaxOffset(deprecatedNode()) && !posRenderedOffset) {
         return false;
     }
     
     if (previousRenderedEditable(deprecatedNode()) == pos.deprecatedNode()
-        && !thisRenderedOffset && posRenderedOffset == (int)caretMaxRenderedOffset(pos.deprecatedNode())) {
+        && !thisRenderedOffset && posRenderedOffset == caretMaxOffset(pos.deprecatedNode())) {
         return false;
     }

trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

@@ -47 +47 @@
 #include "NodeList.h"
 #include "RenderObject.h"
+#include "RenderText.h"
 #include "SmartReplace.h"
 #include "TextIterator.h"
@@ -554 +555 @@
 }
 
+static inline bool nodeHasVisibleRenderText(Text* text)
+{
+    return text->renderer() && toRenderText(text->renderer())->renderedTextLength() > 0;
+}
+
 void ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds()
 {
     document()->updateLayoutIgnorePendingStylesheets();
-    if (!m_lastLeafInserted->renderer()
-        && m_lastLeafInserted->isTextNode()
+    if (m_lastLeafInserted->isTextNode() && !nodeHasVisibleRenderText(static_cast<Text*>(m_lastLeafInserted.get()))
         && !enclosingNodeWithTag(firstPositionInOrBeforeNode(m_lastLeafInserted.get()), selectTag)
         && !enclosingNodeWithTag(firstPositionInOrBeforeNode(m_lastLeafInserted.get()), scriptTag)) {
@@ -574 +579 @@
     // We don't have to make sure that m_firstNodeInserted isn't inside a select or script element, because
     // it is a top level node in the fragment and the user can't insert into those elements.
-    if (!m_firstNodeInserted->renderer() &&
-        m_firstNodeInserted->isTextNode()) {
+    if (m_firstNodeInserted->isTextNode() && !nodeHasVisibleRenderText(static_cast<Text*>(m_firstNodeInserted.get()))) {
         if (m_firstNodeInserted == m_lastLeafInserted) {
             removeNode(m_firstNodeInserted.get());
@@ -607 +611 @@
 VisiblePosition ReplaceSelectionCommand::positionAtStartOfInsertedContent()
 {
-    // Return the inserted content's first VisiblePosition.
-    return VisiblePosition(nextCandidate(positionInParentBeforeNode(m_firstNodeInserted.get())));
+    return firstPositionInOrBeforeNode(m_firstNodeInserted.get());
 }
 

trunk/Source/WebCore/editing/visible_units.cpp

@@ -765 +765 @@
             break;
 
-        if (r->isText() && r->caretMaxRenderedOffset() > 0) {
+        if (r->isText() && toRenderText(r)->renderedTextLength()) {
             ASSERT(n->isTextNode());
             type = Position::PositionIsOffsetInAnchor;
@@ -843 +843 @@
 
         // FIXME: We avoid returning a position where the renderer can't accept the caret.
-        if (r->isText() && r->caretMaxRenderedOffset() > 0) {
+        if (r->isText() && toRenderText(r)->renderedTextLength()) {
             ASSERT(n->isTextNode());
             int length = toRenderText(r)->textLength();

trunk/Source/WebCore/rendering/InlineBox.cpp

@@ -156 +156 @@
 }
 
-unsigned InlineBox::caretMaxRenderedOffset() const 
-{ 
-    return 1; 
-}
-
 void InlineBox::dirtyLineBoxes()
 {

trunk/Source/WebCore/rendering/InlineBox.h

@@ -279 +279 @@
     virtual int caretMinOffset() const;
     virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
 
     unsigned char bidiLevel() const { return m_bidiEmbeddingLevel; }

trunk/Source/WebCore/rendering/InlineTextBox.cpp

@@ -1210 +1210 @@
 }
 
-unsigned InlineTextBox::caretMaxRenderedOffset() const
-{
-    return m_start + m_len;
-}
-
 float InlineTextBox::textPos() const
 {

trunk/Source/WebCore/rendering/InlineTextBox.h

@@ -147 +147 @@
 
 private:
-    virtual unsigned caretMaxRenderedOffset() const;
-
     float textPos() const; // returns the x position relative to the left start of the text line.
 

trunk/Source/WebCore/rendering/RenderBR.cpp

@@ -69 +69 @@
 }
 
-unsigned RenderBR::caretMaxRenderedOffset() const
-{
-    return 1;
-}
-
 VisiblePosition RenderBR::positionForPoint(const LayoutPoint&)
 {

trunk/Source/WebCore/rendering/RenderBR.h

@@ -51 +51 @@
     virtual int caretMinOffset() const;
     virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
 
     virtual VisiblePosition positionForPoint(const LayoutPoint&);

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -2551 +2551 @@
 }
 
-unsigned RenderObject::caretMaxRenderedOffset() const
-{
-    return 0;
-}
-
 int RenderObject::previousOffset(int current) const
 {

trunk/Source/WebCore/rendering/RenderObject.h

@@ -792 +792 @@
     virtual int caretMinOffset() const;
     virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
 
     virtual int previousOffset(int current) const;

trunk/Source/WebCore/rendering/RenderReplaced.cpp

@@ -411 +411 @@
 }
 
-unsigned RenderReplaced::caretMaxRenderedOffset() const
-{
-    return 1; 
-}
-
 VisiblePosition RenderReplaced::positionForPoint(const LayoutPoint& point)
 {

trunk/Source/WebCore/rendering/RenderReplaced.h

@@ -78 +78 @@
     virtual IntRect clippedOverflowRectForRepaint(RenderBoxModelObject* repaintContainer) const;
 
-    virtual unsigned caretMaxRenderedOffset() const;
     virtual VisiblePosition positionForPoint(const LayoutPoint&);
     

trunk/Source/WebCore/rendering/RenderText.cpp

@@ -1596 +1596 @@
 {
     InlineTextBox* box = lastTextBox();
-    if (!box)
+    if (!lastTextBox())
         return textLength();
+
     int maxOffset = box->start() + box->len();
     for (box = box->prevTextBox(); box; box = box->prevTextBox())
@@ -1604 +1605 @@
 }
 
-unsigned RenderText::caretMaxRenderedOffset() const
+unsigned RenderText::renderedTextLength() const
 {
     int l = 0;

trunk/Source/WebCore/rendering/RenderText.h

@@ -108 +108 @@
     virtual int caretMinOffset() const;
     virtual int caretMaxOffset() const;
-    virtual unsigned caretMaxRenderedOffset() const;
+    virtual unsigned renderedTextLength() const;
 
     virtual int previousOffset(int current) const;