Fixed bug when deleting multiple lines

With this fix only one line can be selected in the visual formula
editor, it's not perfect as caret can still be moved to another line
while selecting, which causes the topmost line to be selected. Anyway,
it works and addresses the bug. Better workaround might be possible
later.

3 files changed, 72 insertions, 44 deletions

diff --git a/starmath/source/caret.cxx b/starmath/source/caret.cxx
index 128c427b92b1..a847d136c12f 100644
--- a/starmath/source/caret.cxx
+++ b/starmath/source/caret.cxx
@@ -47,6 +47,10 @@ SmCaretPosGraphEntry* SmCaretPosGraph::Add(SmCaretPosGraphEntry entry){
             pNext = new SmCaretPosGraph();
         return pNext->Add(entry);
     }else{
+        //Set Left and Right to point to the entry itself if they are NULL
+        entry.Left = entry.Left ? entry.Left : Graph + nOffset;
+        entry.Right = entry.Right ? entry.Right : Graph + nOffset;
+        //Save the entry
         Graph[nOffset] = entry;
         return Graph + nOffset++;
     }
diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 5613c3ba3a68..95811bd469c7 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -35,13 +35,13 @@ void SmCursor::Move(OutputDevice* pDev, SmMovementDirection direction, bool bMov
     switch(direction){
         case MoveLeft:
         {
-            //If position->Left is NULL, we want NewPos = NULL anyway...
             NewPos = position->Left;
+            j_assert(NewPos, "NewPos shouldn't be NULL here!");
         }break;
         case MoveRight:
         {
-            //If position->Right is NULL, we want NewPos = NULL anyway...
             NewPos = position->Right;
+            j_assert(NewPos, "NewPos shouldn't be NULL here!");
         }break;
         case MoveUp:
             //Implementation is practically identical to MoveDown, except for a single if statement
@@ -177,8 +177,7 @@ bool SmCursor::SetCaretPosition(SmCaretPos pos, bool moveAnchor){
 
 void SmCursor::AnnotateSelection(){
     //TODO: Manage a state, reset it upon modification and optimize this call
-    SmSetSelectionVisitor SSV(anchor->CaretPos, position->CaretPos);
-    pTree->Accept(&SSV);
+    SmSetSelectionVisitor(anchor->CaretPos, position->CaretPos, pTree);
 }
 
 void SmCursor::Draw(OutputDevice& pDev, Point Offset){
@@ -202,6 +201,7 @@ void SmCursor::Delete(){
 
     //Find the topmost node of the line that holds the selection
     SmNode* pLine = FindTopMostNodeInLine(pSNode, true);
+    j_assert(pLine != pTree, "Shouldn't be able to select the entire tree");
 
     //Get the parent of the line
     SmStructureNode* pLineParent = pLine->GetParent();
@@ -1226,19 +1226,18 @@ SmNode* SmCursor::FindTopMostNodeInLine(SmNode* pSNode, bool MoveUpIfSelected){
         return NULL;
 
     //Move up parent untill we find a node who's
-    //parent isn't selected and not a type of:
+    //parent is NULL or isn't selected and not a type of:
     //      SmExpressionNode
+    //      SmLineNode
     //      SmBinHorNode
     //      SmUnHorNode
     //      SmAlignNode
     //      SmFontNode
-    while((MoveUpIfSelected && pSNode->GetParent()->IsSelected()) ||
-          IsLineCompositionNode(pSNode->GetParent())){
+    while(pSNode->GetParent() &&
+          ((MoveUpIfSelected &&
+            pSNode->GetParent()->IsSelected()) ||
+           IsLineCompositionNode(pSNode->GetParent())))
         pSNode = pSNode->GetParent();
-        j_assert(pSNode, "pSNode shouldn't be NULL, have we hit root node if so, this is bad!");
-        if(!pSNode) //I've got to do something, nothing is probably the best solution :)
-            return NULL;
-    }
     //Now we have the selection line node
     return pSNode;
 }
@@ -1259,6 +1258,7 @@ SmNodeList* SmCursor::LineToList(SmStructureNode* pLine, SmNodeList* list){
     SmNodeIterator it(pLine);
     while(it.Next()){
         switch(it->GetType()){
+            case NLINE:
             case NUNHOR:
             case NEXPRESSION:
             case NBINHOR:
@@ -1304,6 +1304,7 @@ SmNodeList* SmCursor::CloneLineToList(SmStructureNode* pLine, bool bOnlyIfSelect
 
 bool SmCursor::IsLineCompositionNode(SmNode* pNode){
     switch(pNode->GetType()){
+        case NLINE:
         case NUNHOR:
         case NEXPRESSION:
         case NBINHOR:
diff --git a/starmath/source/visitors.cxx b/starmath/source/visitors.cxx
index dc5dd3834f21..c385b3b2d084 100644
--- a/starmath/source/visitors.cxx
+++ b/starmath/source/visitors.cxx
@@ -335,7 +335,6 @@ void SmDefaultingVisitor::Visit( SmVerticalBraceNode* pNode )
     DefaultVisit( pNode );
 }
 
-
 /////////////////////////////// SmCaretDrawingVisitor ////////////////////////////////
 
 SmCaretDrawingVisitor::SmCaretDrawingVisitor( OutputDevice& rDevice,
@@ -435,7 +434,6 @@ void SmCaretPos2LineVisitor::DefaultVisit( SmNode* pNode )
 
 /////////////////////////////// Nasty temporary device!!! ////////////////////////////////
 
-
 #include <tools/gen.hxx>
 #include <tools/fract.hxx>
 #include <rtl/math.hxx>
@@ -470,7 +468,6 @@ public:
     operator OutputDevice & ( ) { return rOutDev; }
 };
 
-
 SmTmpDevice2::SmTmpDevice2( OutputDevice &rTheDev, BOOL bUseMap100th_mm ) :
     rOutDev( rTheDev )
 {
@@ -483,7 +480,6 @@ SmTmpDevice2::SmTmpDevice2( OutputDevice &rTheDev, BOOL bUseMap100th_mm ) :
     }
 }
 
-
 Color SmTmpDevice2::Impl_GetColor( const Color& rColor )
 {
     ColorData nNewCol = rColor.GetColor( );
@@ -509,7 +505,6 @@ Color SmTmpDevice2::Impl_GetColor( const Color& rColor )
     return Color( nNewCol );
 }
 
-
 void SmTmpDevice2::SetFont( const Font &rNewFont )
 {
     rOutDev.SetFont( rNewFont );
@@ -646,7 +641,6 @@ void SmDrawingVisitor::Visit( SmRootSymbolNode* pNode )
     // draw root-sign itself
     DrawSpecialNode( pNode );
 
-
     SmTmpDevice2  aTmpDev( ( OutputDevice & ) rDev, TRUE );
     aTmpDev.SetFillColor( pNode->GetFont( ).GetColor( ) );
     rDev.SetLineColor( );
@@ -809,8 +803,46 @@ void SmDrawingVisitor::DrawChildren( SmNode* pNode )
 
 /////////////////////////////// SmSetSelectionVisitor ////////////////////////////////
 
-void SmSetSelectionVisitor::SetSelectedOnAll( SmNode* pSubTree, bool IsSelected )
-{
+SmSetSelectionVisitor::SmSetSelectionVisitor( SmCaretPos startPos, SmCaretPos endPos, SmNode* pTree) {
+    StartPos    = startPos;
+    EndPos      = endPos;
+    IsSelecting = false;
+
+    //Assume that pTree is a SmTableNode
+    j_assert(pTree->GetType() == NTABLE, "pTree should be a SmTableNode!");
+    //Visit root node, this is special as this node cannot be selected, but it's children can!
+    if(pTree->GetType() == NTABLE){
+        //Change state if StartPos is infront of this node
+        if( StartPos.pSelectedNode == pTree && StartPos.Index == 0 )
+            IsSelecting = !IsSelecting;
+        //Change state if EndPos is infront of this node
+        if( EndPos.pSelectedNode == pTree && EndPos.Index == 0 )
+            IsSelecting = !IsSelecting;
+        j_assert(!IsSelecting, "Caret positions needed to set IsSelecting about, shouldn't be possible!");
+
+        //Visit lines
+        SmNodeIterator it( pTree );
+        while( it.Next( ) ) {
+            it->Accept( this );
+            //If we started a selection in this line and it haven't ended, we do that now!
+            if(IsSelecting) {
+                IsSelecting = false;
+                SetSelectedOnAll(it.Current(), true);
+                //Set StartPos and EndPos to invalid positions, this ensures that an unused
+                //start or end (because we forced end above), doesn't start a new selection.
+                StartPos = EndPos = SmCaretPos();
+            }
+        }
+        //Check if pTree isn't selected
+        j_assert(!pTree->IsSelected(), "pTree should never be selected!");
+        //Discard the selection if there's a bug (it's better than crashing)
+        if(pTree->IsSelected())
+            SetSelectedOnAll(pTree, false);
+    }else //This shouldn't happen, but I don't see any reason to die if it does
+        pTree->Accept(this);
+}
+
+void SmSetSelectionVisitor::SetSelectedOnAll( SmNode* pSubTree, bool IsSelected ) {
     pSubTree->SetSelected( IsSelected );
 
     //Quick BFS to set all selections
@@ -819,8 +851,7 @@ void SmSetSelectionVisitor::SetSelectedOnAll( SmNode* pSubTree, bool IsSelected
         SetSelectedOnAll( it.Current( ), IsSelected );
 }
 
-void SmSetSelectionVisitor::DefaultVisit( SmNode* pNode )
-{
+void SmSetSelectionVisitor::DefaultVisit( SmNode* pNode ) {
     //Change state if StartPos is infront of this node
     if( StartPos.pSelectedNode == pNode && StartPos.Index == 0 )
         IsSelecting = !IsSelecting;
@@ -875,8 +906,7 @@ void SmSetSelectionVisitor::DefaultVisit( SmNode* pNode )
     }
 }
 
-void SmSetSelectionVisitor::VisitCompositionNode( SmNode* pNode )
-{
+void SmSetSelectionVisitor::VisitCompositionNode( SmNode* pNode ) {
     //Change state if StartPos is infront of this node
     if( StartPos.pSelectedNode == pNode && StartPos.Index == 0 )
         IsSelecting = !IsSelecting;
@@ -903,8 +933,7 @@ void SmSetSelectionVisitor::VisitCompositionNode( SmNode* pNode )
         IsSelecting = !IsSelecting;
 }
 
-void SmSetSelectionVisitor::Visit( SmTextNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmTextNode* pNode ) {
     long    i1 = -1,
             i2 = -1;
     if( StartPos.pSelectedNode == pNode )
@@ -946,36 +975,33 @@ void SmSetSelectionVisitor::Visit( SmTextNode* pNode )
     pNode->SetSelectionEnd( end );
 }
 
-void SmSetSelectionVisitor::Visit( SmExpressionNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmExpressionNode* pNode ) {
     VisitCompositionNode( pNode );
 }
 
-void SmSetSelectionVisitor::Visit( SmAlignNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmLineNode* pNode ) {
     VisitCompositionNode( pNode );
 }
 
-void SmSetSelectionVisitor::Visit( SmBinHorNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmAlignNode* pNode ) {
     VisitCompositionNode( pNode );
 }
 
-void SmSetSelectionVisitor::Visit( SmUnHorNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmBinHorNode* pNode ) {
     VisitCompositionNode( pNode );
 }
 
-void SmSetSelectionVisitor::Visit( SmFontNode* pNode )
-{
+void SmSetSelectionVisitor::Visit( SmUnHorNode* pNode ) {
     VisitCompositionNode( pNode );
 }
 
-
+void SmSetSelectionVisitor::Visit( SmFontNode* pNode ) {
+    VisitCompositionNode( pNode );
+}
 
 /////////////////////////////// SmCaretPosGraphBuildingVisitor ////////////////////////////////
 
-SmCaretPosGraphBuildingVisitor::SmCaretPosGraphBuildingVisitor( SmNode* pRootNode ){
+SmCaretPosGraphBuildingVisitor::SmCaretPosGraphBuildingVisitor( SmNode* pRootNode ) {
     pRightMost  = NULL;
     pGraph = new SmCaretPosGraph( );
     //pRootNode should always be a table
@@ -995,7 +1021,7 @@ SmCaretPosGraphBuildingVisitor::SmCaretPosGraphBuildingVisitor( SmNode* pRootNod
             //The argument for doing this is that we now don't have to worry about SmLineNode
             //being a visual line composition node. Thus, no need for yet another special case
             //in SmCursor::IsLineCompositionNode and everywhere this method is used.
-            if( it->GetType( ) != NLINE )
+            //if( it->GetType( ) != NLINE )
                 pRightMost = pGraph->Add( SmCaretPos( it.Current( ), 0 ) );
             it->Accept( this );
         }
@@ -1004,11 +1030,11 @@ SmCaretPosGraphBuildingVisitor::SmCaretPosGraphBuildingVisitor( SmNode* pRootNod
 }
 
 void SmCaretPosGraphBuildingVisitor::Visit( SmLineNode* pNode ){
-    pRightMost = NULL;
+    //pRightMost = NULL;
     SmNodeIterator it( pNode );
     while( it.Next( ) ){
-        if( !pRightMost )
-            pRightMost = pGraph->Add( SmCaretPos( it.Current( ), 0 ) );
+        //if( !pRightMost )
+        //    pRightMost = pGraph->Add( SmCaretPos( it.Current( ), 0 ) );
         it->Accept( this );
     }
 }
@@ -1489,7 +1515,6 @@ void SmCaretPosGraphBuildingVisitor::Visit( SmBinDiagonalNode* pNode )
     pRightMost = right;
 }
 
-
 //Straigt forward ( I think )
 void SmCaretPosGraphBuildingVisitor::Visit( SmBinHorNode* pNode )
 {
@@ -1853,7 +1878,6 @@ void SmCloningVisitor::Visit( SmBraceNode* pNode )
     pResult = pClone;
 }
 
-
 void SmCloningVisitor::Visit( SmBracebodyNode* pNode )
 {
     SmBracebodyNode* pClone = new SmBracebodyNode( pNode->GetToken( ) );
@@ -2114,7 +2138,6 @@ void SmSelectionDrawingVisitor::Visit( SmTextNode* pNode )
     }
 }
 
-
 /////////////////////////////// SmNodeToTextVisitor ///////////////////////////////
 
 void SmNodeToTextVisitor::Visit( SmTableNode* pNode )