[webkit-reviews] review granted: [Bug 36037] REGRESSION(51522): typing at the end of a line in designMode documents is *very* slow : [Attachment 56949] Patch2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 36037: REGRESSION(51522): typing at the end of a line in designMode
documents is *very* slow
https://bugs.webkit.org/show_bug.cgi?id=36037

Attachment 56949: Patch2
https://bugs.webkit.org/attachment.cgi?id=56949&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   m_offsetInAnchor = (m_anchorNode->hasChildNodes())? 0:
lastOffsetForEditing(m_anchorNode);

Formatting nitpick: Please write it like this:

    m_offsetInAnchor = m_anchorNode->hasChildNodes() ? 0:
lastOffsetForEditing(m_anchorNode);

But also, it seems wasteful to call lastOffsetForEditing in a case where we
already know the node is non-null and we know that hasChildNodes is false. The
special editingIgnoresContent behavior is the main reason we have the
lastOffsetForEditing function. If we're not trying to implement that rule, then
I think the code above is equivalent to this:

    m_offsetInAnchor = m_anchorNode->offsetInCharacters() ?
m_anchorNode->maxCharacterOffset() : 0;

For efficiency we could optimize it like this:

    m_offsetInAnchor = (m_anchorNode->hasChildNodes() ||
!m_anchorNode->offsetInCharacters()) ? 0 : m_anchorNode->maxCharacterOffset();

I'm not sure which is the best way to write it, but I do think that calling
lastOffsetForEditing is a little strange. On the other hand, using it
consistently might be clearer from a code readability point of view.

r=me even if you don't agree with my comments. It's OK to just fix the
formatting nitpick.