[webkit-reviews] review granted: [Bug 68875] Encapsulate m_firstNodeInserted and m_lastLeafInserted in node insertion logic : [Attachment 108802] Added the forgotten DeleteSelectionCommand change

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 68875: Encapsulate m_firstNodeInserted and m_lastLeafInserted in node
insertion logic
https://bugs.webkit.org/show_bug.cgi?id=68875

Attachment 108802: Added the forgotten DeleteSelectionCommand change
https://bugs.webkit.org/attachment.cgi?id=108802&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108802&action=review


> Source/WebCore/editing/ReplaceSelectionCommand.h:82
> +	   void respondToNodeInsertion(Node* node)
> +	   {
> +	       if (!node)
> +		   return;
> +	       
> +	       if (!m_firstNodeInserted)
> +		   m_firstNodeInserted = node;
> +	       
> +	       m_lastNodeInserted = node;
> +	   }
> +	   void willRemoveNode(Node* node)
> +	   {
> +	       if (m_firstNodeInserted == node)
> +		   m_firstNodeInserted = node->traverseNextNode();
> +	       if (m_lastNodeInserted == node)
> +		   m_lastNodeInserted = node->lastChild() ? node->lastChild() :
node->traverseNextSibling();
> +	   }

nit: I'd like to move these function definitions to ReplaceSelectionCommand.cpp
in order to reduce the header size.

> Source/WebCore/editing/ReplaceSelectionCommand.h:85
> +	   Node* firstNodeInserted() { return m_firstNodeInserted.get(); }
> +	   Node* lastLeafInserted() { return
m_lastNodeInserted->lastDescendant(); }
> +	   Node* pastLastLeaf() { return m_firstNodeInserted ?
lastLeafInserted()->traverseNextNode() : 0; }

nit: These function should be "const".

> Source/WebCore/editing/ReplaceSelectionCommand.h:86
> +    private:

Need a blank line before the private: label.