Reviewed by Darin.

        <rdar://problem/5415006> Command Left in a To Do causes caret to disappear

        The selection was ending up inside non-editable content at the To Do Options
        arrow image, rather then at the editable position just to the left of that image.
        The problem was that startPositionForLine looked only at line boxes, and there
        is no linebox for the editable position at the far left of a To Do, which is
        a table. Addressed by having startPositionForLine use table offset 0 instead
        of the first VisiblePosition inside the table.

        Found and fixed the similar case with option-left (move by word position).

        Test cases:
        * editing/selection/mixed-editability-8.html: Added.
        * editing/selection/mixed-editability-9.html: Added.

        Source changes:
        * editing/SelectionController.cpp:
        (WebCore::SelectionController::modifyMovingLeftBackward):

        * editing/VisiblePosition.cpp:
        (WebCore::VisiblePosition::next):
        (WebCore::VisiblePosition::previous):
        (WebCore::VisiblePosition::stayInEditableContentLeft):
        (WebCore::VisiblePosition::stayInEditableContentRight):
        Factored stayInEditableContentLeft() and stayInEditableContentRight()
        out of previous() and next().

        * editing/VisiblePosition.h:
        Declare stayInEditableContentLeft() and stayInEditableContentRight().

        * editing/visible_units.cpp:
        (WebCore::previousWordPosition):
        (WebCore::nextWordPosition):
        (WebCore::startOfLine):
        (WebCore::endOfLine):
        (WebCore::previousSentencePosition):
        (WebCore::nextSentencePosition):
        Call stayInEditableContentLeft() or stayInEditableContentRight(), as
        appropriate, so prevent crossing from editable content into
        uneditable content.

        (WebCore::startPositionForLine):
        Use table offset 0 instead of the first VisiblePosition in the table.

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

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index a89ee6dc96b4de8b1e752b6ac10fdc6d35a3f15f..2513eb7a9a47ec8dfb500a901a7ee99bbc0f7e86 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,51 @@
+2007-08-28  David Harrison  <harrison@apple.com>
+
+        Reviewed by Darin.
+
+        <rdar://problem/5415006> Command Left in a To Do causes caret to disappear
+
+        The selection was ending up inside non-editable content at the To Do Options
+        arrow image, rather then at the editable position just to the left of that image.
+        The problem was that startPositionForLine looked only at line boxes, and there
+        is no linebox for the editable position at the far left of a To Do, which is
+        a table. Addressed by having startPositionForLine use table offset 0 instead
+        of the first VisiblePosition inside the table.
+        
+        Found and fixed the similar case with option-left (move by word position).
+        
+        Test cases:
+        * editing/selection/mixed-editability-8.html: Added.
+        * editing/selection/mixed-editability-9.html: Added.
+
+        Source changes:
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::modifyMovingLeftBackward):
+        
+        * editing/VisiblePosition.cpp:
+        (WebCore::VisiblePosition::next):
+        (WebCore::VisiblePosition::previous):
+        (WebCore::VisiblePosition::stayInEditableContentLeft):
+        (WebCore::VisiblePosition::stayInEditableContentRight):
+        Factored stayInEditableContentLeft() and stayInEditableContentRight()
+        out of previous() and next().
+        
+        * editing/VisiblePosition.h:
+        Declare stayInEditableContentLeft() and stayInEditableContentRight().
+
+        * editing/visible_units.cpp:
+        (WebCore::previousWordPosition):
+        (WebCore::nextWordPosition):
+        (WebCore::startOfLine):
+        (WebCore::endOfLine):
+        (WebCore::previousSentencePosition):
+        (WebCore::nextSentencePosition):
+        Call stayInEditableContentLeft() or stayInEditableContentRight(), as 
+        appropriate, so prevent crossing from editable content into
+        uneditable content.
+        
+        (WebCore::startPositionForLine):
+        Use table offset 0 instead of the first VisiblePosition in the table.
+        
 2007-08-28  Mark Rowe  <mrowe@apple.com>
 
         Reviewed by Darin Adler.

diff --git a/WebCore/editing/SelectionController.cpp b/WebCore/editing/SelectionController.cpp
index 6d3cdb844201e2473688b4c3cd05d282f8f4d0a0..19d34317695d6e0d2cfbdeeb25fcb6a69781e982 100644 (file)
--- a/WebCore/editing/SelectionController.cpp
+++ b/WebCore/editing/SelectionController.cpp
@@ -368,7 +368,6 @@ VisiblePosition SelectionController::modifyExtendingLeftBackward(TextGranularity
 VisiblePosition SelectionController::modifyMovingLeftBackward(TextGranularity granularity)
 {
     VisiblePosition pos;
-    // FIXME: Stay in editable content for the less common granularities.
     switch (granularity) {
         case CharacterGranularity:
             if (isRange()) 

diff --git a/WebCore/editing/VisiblePosition.cpp b/WebCore/editing/VisiblePosition.cpp
index 5e95838e8cc2ae72983ba91a3002f1caf4dce3a8..a325bfb389d6c6450bdf6a1b5eeaa056cb0da287 100644 (file)
--- a/WebCore/editing/VisiblePosition.cpp
+++ b/WebCore/editing/VisiblePosition.cpp
@@ -66,18 +66,10 @@ VisiblePosition VisiblePosition::next(bool stayInEditableContent) const
 {
     VisiblePosition next(nextVisuallyDistinctCandidate(m_deepPosition), m_affinity);
     
-    if (!stayInEditableContent || next.isNull())
+    if (!stayInEditableContent)
         return next;
     
-    Node* highestRoot = highestEditableRoot(deepEquivalent());
-    
-    if (!next.deepEquivalent().node()->isDescendantOf(highestRoot))
-        return VisiblePosition();
-
-    if (highestEditableRoot(next.deepEquivalent()) == highestRoot)
-        return next;
-    
-    return firstEditablePositionAfterPositionInRoot(next.deepEquivalent(), highestRoot);
+    return firstEditablePositionAtOrAfter(next);
 }
 
 VisiblePosition VisiblePosition::previous(bool stayInEditableContent) const
@@ -102,18 +94,42 @@ VisiblePosition VisiblePosition::previous(bool stayInEditableContent) const
     }
 #endif
 
-    if (!stayInEditableContent || prev.isNull())
+    if (!stayInEditableContent)
         return prev;
     
+    return lastEditablePositionAtOrBefore(prev);
+}
+
+VisiblePosition VisiblePosition::lastEditablePositionAtOrBefore(const VisiblePosition &pos) const
+{
+    if (pos.isNull())
+        return pos;
+    
     Node* highestRoot = highestEditableRoot(deepEquivalent());
     
-    if (!prev.deepEquivalent().node()->isDescendantOf(highestRoot))
+    if (!pos.deepEquivalent().node()->isDescendantOf(highestRoot))
         return VisiblePosition();
         
-    if (highestEditableRoot(prev.deepEquivalent()) == highestRoot)
-        return prev;
+    if (highestEditableRoot(pos.deepEquivalent()) == highestRoot)
+        return pos;
+
+    return lastEditablePositionBeforePositionInRoot(pos.deepEquivalent(), highestRoot);
+}
+
+VisiblePosition VisiblePosition::firstEditablePositionAtOrAfter(const VisiblePosition &pos) const
+{
+    if (pos.isNull())
+        return pos;
+    
+    Node* highestRoot = highestEditableRoot(deepEquivalent());
+    
+    if (!pos.deepEquivalent().node()->isDescendantOf(highestRoot))
+        return VisiblePosition();
+        
+    if (highestEditableRoot(pos.deepEquivalent()) == highestRoot)
+        return pos;
 
-    return lastEditablePositionBeforePositionInRoot(prev.deepEquivalent(), highestRoot);
+    return firstEditablePositionAfterPositionInRoot(pos.deepEquivalent(), highestRoot);
 }
 
 Position canonicalizeCandidate(const Position& candidate)

diff --git a/WebCore/editing/VisiblePosition.h b/WebCore/editing/VisiblePosition.h
index 2a58ee9a59dfe7084b853da120014ed1a31a751d..64ebd61047e90ba3617458b82ac7ccad2233689a 100644 (file)
--- a/WebCore/editing/VisiblePosition.h
+++ b/WebCore/editing/VisiblePosition.h
@@ -63,6 +63,8 @@ public:
     // next() and previous() will increment/decrement by a character cluster.
     VisiblePosition next(bool stayInEditableContent = false) const;
     VisiblePosition previous(bool stayInEditableContent = false) const;
+    VisiblePosition lastEditablePositionAtOrBefore(const VisiblePosition&) const;
+    VisiblePosition firstEditablePositionAtOrAfter(const VisiblePosition&) const;
 
     UChar characterAfter() const;
     UChar characterBefore() const { return previous().characterAfter(); }
@@ -87,17 +89,17 @@ private:
 };
 
 // FIXME: This shouldn't ignore affinity.
-inline bool operator==(const VisiblePosition &a, const VisiblePosition &b)
+inline bool operator==(const VisiblePosition& a, const VisiblePosition& b)
 {
     return a.deepEquivalent() == b.deepEquivalent();
 }
  
-inline bool operator!=(const VisiblePosition &a, const VisiblePosition &b)
+inline bool operator!=(const VisiblePosition& a, const VisiblePosition& b)
 {
     return !(a == b);
 }
 
-PassRefPtr<Range> makeRange(const VisiblePosition &, const VisiblePosition &);
+PassRefPtr<Range> makeRange(const VisiblePosition&, const VisiblePosition&);
 bool setStart(Range*, const VisiblePosition&);
 bool setEnd(Range*, const VisiblePosition&);
 VisiblePosition startVisiblePosition(const Range*, EAffinity);

diff --git a/WebCore/editing/visible_units.cpp b/WebCore/editing/visible_units.cpp
index 35d17788524441be72a59d34e92ee498e747bed6..008c05f74d7deb03d992743fb98879a3d825445a 100644 (file)
--- a/WebCore/editing/visible_units.cpp
+++ b/WebCore/editing/visible_units.cpp
@@ -235,7 +235,8 @@ static unsigned previousWordPositionBoundary(const UChar* characters, unsigned l
 
 VisiblePosition previousWordPosition(const VisiblePosition &c)
 {
-    return previousBoundary(c, previousWordPositionBoundary);
+    VisiblePosition prev = previousBoundary(c, previousWordPositionBoundary);
+    return c.firstEditablePositionAtOrAfter(prev);
 }
 
 static unsigned nextWordPositionBoundary(const UChar* characters, unsigned length)
@@ -245,7 +246,8 @@ static unsigned nextWordPositionBoundary(const UChar* characters, unsigned lengt
 
 VisiblePosition nextWordPosition(const VisiblePosition &c)
 {
-    return nextBoundary(c, nextWordPositionBoundary);
+    VisiblePosition next = nextBoundary(c, nextWordPositionBoundary);    
+    return c.lastEditablePositionAtOrBefore(next);
 }
 
 // ---------
@@ -309,16 +311,22 @@ static VisiblePosition startPositionForLine(const VisiblePosition& c)
         InlineTextBox *startTextBox = static_cast<InlineTextBox *>(startBox);
         startOffset = startTextBox->m_start;
     }
-    
-    return VisiblePosition(startNode, startOffset, DOWNSTREAM);
+  
+    VisiblePosition visPos = VisiblePosition(startNode, startOffset, DOWNSTREAM);
+
+    // return table offset 0 instead of the first VisiblePosition inside the table
+    VisiblePosition visPrevious = visPos.previous();
+    if (isLastPositionBeforeTable(visPrevious))
+        visPos = visPrevious;
+
+    return visPos;
 }
 
 VisiblePosition startOfLine(const VisiblePosition& c)
 {
     VisiblePosition visPos = startPositionForLine(c);
     
-    if (visPos.isNotNull())
-    {
+    if (visPos.isNotNull()) {
         // Make sure the start of line is not greater than the given input position.  Else use the previous position to 
         // obtain start of line.  This condition happens when the input position is before the space character at the end 
         // of a soft-wrapped non-editable line. In this scenario, startPositionForLine would incorrectly hand back a position
@@ -332,8 +340,8 @@ VisiblePosition startOfLine(const VisiblePosition& c)
             visPos = startPositionForLine(visPos);
         }
     }
-    
-    return visPos;
+
+    return c.firstEditablePositionAtOrAfter(visPos);
 }
 
 static VisiblePosition endPositionForLine(const VisiblePosition& c)
@@ -400,7 +408,7 @@ VisiblePosition endOfLine(const VisiblePosition& c)
         visPos = endPositionForLine(visPos);
     }
     
-    return visPos;
+    return c.lastEditablePositionAtOrBefore(visPos);
 }
 
 bool inSameLine(const VisiblePosition &a, const VisiblePosition &b)
@@ -595,7 +603,8 @@ static unsigned previousSentencePositionBoundary(const UChar* characters, unsign
 
 VisiblePosition previousSentencePosition(const VisiblePosition &c)
 {
-    return previousBoundary(c, previousSentencePositionBoundary);
+    VisiblePosition prev = previousBoundary(c, previousSentencePositionBoundary);
+    return c.firstEditablePositionAtOrAfter(prev);
 }
 
 static unsigned nextSentencePositionBoundary(const UChar* characters, unsigned length)
@@ -608,7 +617,8 @@ static unsigned nextSentencePositionBoundary(const UChar* characters, unsigned l
 
 VisiblePosition nextSentencePosition(const VisiblePosition &c)
 {
-    return nextBoundary(c, nextSentencePositionBoundary);
+    VisiblePosition next = nextBoundary(c, nextSentencePositionBoundary);    
+    return c.lastEditablePositionAtOrBefore(next);
 }
 
 // FIXME: Broken for positions before/after images that aren't inline (5027702)