Invalid caret at the end of line in overtype mode

Source/core/editing/FrameSelection.cpp

 /*
  * Copyright (C) 2004, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1516 matching lines @@
 
 inline static bool shouldStopBlinkingDueToTypingCommand(LocalFrame* frame)
 {
     return frame->editor().lastEditCommand() && frame->editor().lastEditCommand()->shouldStopCaretBlinking();
 }
 
 void FrameSelection::updateAppearance()
 {
     // Paint a block cursor instead of a caret in overtype mode unless the caret is at the end of a line (in this case
     // the FrameSelection will paint a blinking caret as usual).
-    VisiblePosition forwardPosition;
-    if (m_shouldShowBlockCursor && m_selection.isCaret()) {
-        forwardPosition = modifyExtendingForward(CharacterGranularity);
-        m_caretPaint = forwardPosition.isNull();
-    }
+    bool paintBlockCursor = m_shouldShowBlockCursor && m_selection.isCaret() && !isLogicalEndOfLine(endForPlatform());

[leviw_travelin_and_unemployed, 2014/08/07 18:39:16]

Since we're only using it if it's a caret, endForPlatform() doesn't actually do
anything in this case, right? I think having it in there makes it more
confusing.

[svillar, 2014/08/08 09:01:25]

Indeed, you're totally right.

 
     bool caretRectChangedOrCleared = recomputeCaretRect();
-    bool shouldBlink = shouldBlinkCaret() && forwardPosition.isNull();
+    bool shouldBlink = !paintBlockCursor && shouldBlinkCaret();
 
     // If the caret moved, stop the blink timer so we can restart with a
     // black caret in the new location.
     if (caretRectChangedOrCleared || !shouldBlink || shouldStopBlinkingDueToTypingCommand(m_frame)) {
         m_caretBlinkTimer.stop();
         if (!shouldBlink && m_caretPaint) {
             m_caretPaint = false;
             invalidateCaretRect();
         }
     }
 
     // Start blinking with a black caret. Be sure not to restart if we're
     // already blinking in the right location.
     if (shouldBlink && !m_caretBlinkTimer.isActive()) {
         if (double blinkInterval = RenderTheme::theme().caretBlinkInterval())
             m_caretBlinkTimer.startRepeating(blinkInterval, FROM_HERE);
 
         if (!m_caretPaint) {
             m_caretPaint = true;
             invalidateCaretRect();
         }
     }
 
     RenderView* view = m_frame->contentRenderer();
     if (!view)
         return;
 
     // Construct a new VisibleSolution, since m_selection is not necessarily valid, and the following steps
     // assume a valid selection. See <https://bugs.webkit.org/show_bug.cgi?id=69563> and <rdar://problem/10232866>.
-    VisibleSelection selection(m_selection.visibleStart(), forwardPosition.isNotNull() ? forwardPosition : m_selection.visibleEnd());
+    VisiblePosition endVisiblePosition = paintBlockCursor ? modifyExtendingForward(CharacterGranularity) : m_selection.visibleEnd();
+    VisibleSelection selection(m_selection.visibleStart(), endVisiblePosition);
 
     if (!selection.isRange()) {
         view->clearSelection();
         return;
     }
 
     // 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'.
@@ skipping 333 matching lines @@
     sel.showTreeForThis();
 }
 
 void showTree(const blink::FrameSelection* sel)
 {
     if (sel)
         sel->showTreeForThis();
 }
 
 #endif