2006-10-03 Darin Adler <darin@apple.com>
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Oct 2006 01:06:57 +0000 (01:06 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Oct 2006 01:06:57 +0000 (01:06 +0000)
        Reviewed by Justin Garcia.

        - correct mistaken code that would restart blinking every
          time "invalidateSelection" is called even if the caret
          has not moved

        * editing/SelectionController.h:
        * editing/SelectionController.cpp:
        (WebCore::SelectionController::setSelection): Update for name change.
        (WebCore::SelectionController::recomputeCaretRect): New function.
        Computes caret rect and does any necessary invalidation if the rect
        changes.
        (WebCore::SelectionController::invalidateCaretRect): Renamed from
        needsCaretRepaint. Invalidates the caret rect unconditionally. Also
        calls recomputeCaretRect as a side effect.

        * page/Frame.cpp:
        (WebCore::Frame::invalidateSelection): Remove unneeded call to
        clearCaretRectIfNeeded.
        (WebCore::Frame::clearCaretRectIfNeeded): Updated for name change.
        (WebCore::Frame::selectionLayoutChanged): Restructured to use the new
        recomputeCaretRect function and not restart blinking if caret has
        not changed position.
        (WebCore::Frame::caretBlinkTimerFired): Removed some checks that
        are not needed since selectionLayoutChanged already checks these.

        * page/FramePrivate.h: Removed unused m_blinkCaret.

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

WebCore/ChangeLog
WebCore/editing/SelectionController.cpp
WebCore/editing/SelectionController.h
WebCore/page/Frame.cpp
WebCore/page/FramePrivate.h

index cfea93d..a18e2f4 100644 (file)
@@ -1,3 +1,33 @@
+2006-10-03  Darin Adler  <darin@apple.com>
+
+        Reviewed by Justin Garcia.
+
+        - correct mistaken code that would restart blinking every
+          time "invalidateSelection" is called even if the caret
+          has not moved
+
+        * editing/SelectionController.h:
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::setSelection): Update for name change.
+        (WebCore::SelectionController::recomputeCaretRect): New function.
+        Computes caret rect and does any necessary invalidation if the rect
+        changes.
+        (WebCore::SelectionController::invalidateCaretRect): Renamed from
+        needsCaretRepaint. Invalidates the caret rect unconditionally. Also
+        calls recomputeCaretRect as a side effect.
+
+        * page/Frame.cpp:
+        (WebCore::Frame::invalidateSelection): Remove unneeded call to
+        clearCaretRectIfNeeded.
+        (WebCore::Frame::clearCaretRectIfNeeded): Updated for name change.
+        (WebCore::Frame::selectionLayoutChanged): Restructured to use the new
+        recomputeCaretRect function and not restart blinking if caret has
+        not changed position.
+        (WebCore::Frame::caretBlinkTimerFired): Removed some checks that
+        are not needed since selectionLayoutChanged already checks these.
+
+        * page/FramePrivate.h: Removed unused m_blinkCaret.
+
 2006-10-02  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Adam.
index 839d6f0..c012b43 100644 (file)
@@ -83,10 +83,10 @@ void SelectionController::moveTo(const Position &base, const Position &extent, E
 void SelectionController::setSelection(const Selection& s, bool closeTyping, bool clearTypingStyle, bool userTriggered)
 {
     if (m_isDragCaretController) {
-        needsCaretRepaint();
+        invalidateCaretRect();
         m_sel = s;
         m_needsLayout = true;
-        needsCaretRepaint();
+        invalidateCaretRect();
         return;
     }
     if (!m_frame) {
@@ -792,34 +792,55 @@ IntRect SelectionController::caretRepaintRect() const
     return IntRect(r.x() - 1, r.y() - 1, r.width() + 2, r.height() + 2);
 }
 
-void SelectionController::needsCaretRepaint()
+bool SelectionController::recomputeCaretRect()
+{
+    if (!isCaret())
+        return false;
+
+    FrameView* v = m_sel.start().node()->document()->view();
+    if (!v)
+        return false;
+
+    if (!m_needsLayout)
+        return false;
+
+    IntRect oldRect = caretRepaintRect();
+    layout();
+    IntRect newRect = caretRepaintRect();
+    if (oldRect == newRect)
+        return false;
+
+    v->updateContents(oldRect, false);
+    v->updateContents(newRect, false);
+    return true;
+}
+
+void SelectionController::invalidateCaretRect()
 {
     if (!isCaret())
         return;
 
-    FrameView *v = m_sel.start().node()->document()->view();
+    FrameViewv = m_sel.start().node()->document()->view();
     if (!v)
         return;
 
-    if (m_needsLayout) {
-        // repaint old position and calculate new position
+    bool caretRectChanged = recomputeCaretRect();
+
+    // EDIT FIXME: This is an unfortunate hack.
+    // Basically, we can't trust this layout position since we 
+    // can't guarantee that the check to see if we are in unrendered 
+    // content will work at this point. We may have to wait for
+    // a layout and re-render of the document to happen. So, resetting this
+    // flag will cause another caret layout to happen the first time
+    // that we try to paint the caret after this call. That one will work since
+    // it happens after the document has accounted for any editing
+    // changes which may have been done.
+    // And, we need to leave this layout here so the caret moves right 
+    // away after clicking.
+    m_needsLayout = true;
+
+    if (!caretRectChanged)
         v->updateContents(caretRepaintRect(), false);
-        layout();
-        
-        // EDIT FIXME: This is an unfortunate hack.
-        // Basically, we can't trust this layout position since we 
-        // can't guarantee that the check to see if we are in unrendered 
-        // content will work at this point. We may have to wait for
-        // a layout and re-render of the document to happen. So, resetting this
-        // flag will cause another caret layout to happen the first time
-        // that we try to paint the caret after this call. That one will work since
-        // it happens after the document has accounted for any editing
-        // changes which may have been done.
-        // And, we need to leave this layout here so the caret moves right 
-        // away after clicking.
-        m_needsLayout = true;
-    }
-    v->updateContents(caretRepaintRect(), false);
 }
 
 void SelectionController::paintCaret(GraphicsContext *p, const IntRect &rect)
index c7e8373..1a45d93 100644 (file)
@@ -29,6 +29,7 @@
 #include "IntRect.h"
 #include "Selection.h"
 #include "Range.h"
+#include <wtf/Noncopyable.h>
 
 namespace WebCore {
 
@@ -37,14 +38,12 @@ class GraphicsContext;
 class RenderObject;
 class VisiblePosition;
 
-class SelectionController
-{
+class SelectionController : Noncopyable {
 public:
     enum EAlter { MOVE, EXTEND };
     enum EDirection { FORWARD, BACKWARD, RIGHT, LEFT };
-#define SEL_DEFAULT_AFFINITY DOWNSTREAM
 
-    SelectionController(Frame* frame = 0, bool isDragCaretController = false);
+    SelectionController(Frame* = 0, bool isDragCaretController = false);
 
     Element* rootEditableElement() const { return selection().rootEditableElement(); }
     bool isContentEditable() const { return selection().isContentEditable(); }
@@ -136,8 +135,9 @@ public:
     //void clear();
     //TextRange *createRange();
     
-    void needsCaretRepaint();
-    void paintCaret(GraphicsContext*, const IntRect &rect);
+    bool recomputeCaretRect(); // returns true if caret rect moved
+    void invalidateCaretRect();
+    void paintCaret(GraphicsContext*, const IntRect&);
 
 #ifndef NDEBUG
     void formatForDebugger(char* buffer, unsigned length) const;
@@ -145,9 +145,6 @@ public:
 #endif
 
 private:
-    SelectionController(const SelectionController&);
-    SelectionController& operator=(const SelectionController&);
-
     enum EPositionType { START, END, BASE, EXTENT };
 
     VisiblePosition modifyExtendingRightForward(TextGranularity);
index afd2f36..fa885a7 100644 (file)
@@ -1283,7 +1283,6 @@ void Frame::notifyRendererOfSelectionChange(bool userTriggered)
 
 void Frame::invalidateSelection()
 {
-    clearCaretRectIfNeeded();
     selectionController()->setNeedsLayout();
     selectionLayoutChanged();
 }
@@ -1304,7 +1303,7 @@ void Frame::clearCaretRectIfNeeded()
 {
     if (d->m_caretPaint) {
         d->m_caretPaint = false;
-        selectionController()->needsCaretRepaint();
+        selectionController()->invalidateCaretRect();
     }        
 }
 
@@ -1350,15 +1349,21 @@ void Frame::setFocusNodeIfNeeded()
 
 void Frame::selectionLayoutChanged()
 {
-    // kill any caret blink timer now running
-    d->m_caretBlinkTimer.stop();
+    bool caretRectChanged = selectionController()->recomputeCaretRect();
+
+    bool shouldBlink = d->m_caretVisible
+        && selectionController()->isCaret() && selectionController()->isContentEditable();
 
-    // see if a new caret blink timer needs to be started
-    if (d->m_caretVisible && d->m_caretBlinks && 
-        selectionController()->isCaret() && selectionController()->start().node()->isContentEditable()) {
+    // If the caret moved, stop the blink timer so we can restart with a
+    // black caret in the new location.
+    if (caretRectChanged || !shouldBlink)
+        d->m_caretBlinkTimer.stop();
+
+    // Start blinking with a black caret. Be sure not to restart if we're
+    // already blinking in the right location.
+    if (shouldBlink && !d->m_caretBlinkTimer.isActive()) {
         d->m_caretBlinkTimer.startRepeating(caretBlinkFrequency);
         d->m_caretPaint = true;
-        selectionController()->needsCaretRepaint();
     }
 
     if (d->m_doc)
@@ -1377,21 +1382,13 @@ int Frame::xPosForVerticalArrowNavigation() const
 
 void Frame::caretBlinkTimerFired(Timer<Frame>*)
 {
-    // Might be better to turn the timer off during some of these circumstances
-    // and assert rather then letting the timer fire and do nothing here.
-    // Could do that in selectionLayoutChanged.
-
-    if (!d->m_caretVisible)
-        return;
-    if (!d->m_caretBlinks)
-        return;
-    if (!selectionController()->isCaret())
-        return;
+    ASSERT(d->m_caretVisible);
+    ASSERT(selectionController()->isCaret());
     bool caretPaint = d->m_caretPaint;
     if (d->m_bMousePressed && caretPaint)
         return;
     d->m_caretPaint = !caretPaint;
-    selectionController()->needsCaretRepaint();
+    selectionController()->invalidateCaretRect();
 }
 
 void Frame::paintCaret(GraphicsContext* p, const IntRect& rect) const
index d9c2ebf..6ae0566 100644 (file)
@@ -88,7 +88,6 @@ namespace WebCore {
             , m_caretBlinkTimer(thisFrame, &Frame::caretBlinkTimerFired)
             , m_command(thisFrame)
             , m_caretVisible(false)
-            , m_caretBlinks(true)
             , m_caretPaint(true)
             , m_bFirstData(true)
             , m_bCleared(true)
@@ -198,7 +197,6 @@ namespace WebCore {
         CommandByName m_command;
 
         bool m_caretVisible : 1;
-        bool m_caretBlinks : 1;
         bool m_caretPaint : 1;
         bool m_bFirstData : 1;
         bool m_bCleared : 1;