View Issue Details

IDProjectCategoryView StatusLast Update
0009867ScribusUndo/Redopublic2011-03-27 14:34
Reportercezaryece Assigned Tojghali  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformall 
Product Version1.4.0svn 
Fixed in Version1.4.0svn 
Summary0009867: [BUG] text undo strange behaviour for line spacing changes
DescriptionAfter changes in PageItem_TextFrame::undoUpdate made by jghali to restoring text selection new issues appears as my text undo solution is based on selecting text for storing it state before changes.
this selection should be temporary and passed to updateUndo, but now for changes for whole paragraphs (like line spacing) it is working by selecting paragraph before changes, storing it in itemTextSaxed and call updateUndo with text selected. UpdateUndo take selection start and lenght and store it in undo state.
If selection is changed between these two processes result is wrong and undo state is broken.
I have patch for fix it ready, but must wait for commit patch from 0009865.

In fact changes are not big.
It is only uncommenting two rows in propertiespalette.cpp with same line
"CurItem->asTextFrame()->ExpandParSel();"
1. in PropertiesPalette::setLineSpacingMode(int id)
2. in PropertiesPalette::NewLineSpacing()
TagsNo tags attached.
Patch

Activities

cezaryece

2011-03-22 11:16

updater   ~0025886

Ups.. I am wrong... not only these two lines.
It is also change in PageItem_TextFrame::updateUndo
where begin should be

void PageItem_TextFrame::updateUndo(EditAct action, QString str, QString oldstr)
{
    if (UndoManager::undoEnabled() && undoManager->undoEnabled())
    {
        int oldSelStart = -1, oldSelLen = -1;
        oldSelLen = itemText.lengthOfSelection();
        if (oldSelLen > 0)
            oldSelStart = itemText.startOfSelection();
        if (m_Doc->appMode == modeEdit && action == PARAMFULL && oldSelLen == 0 && CPos < itemText.length() && itemText.itemText(CPos) == SpecialChars::PARSEP)
            //case of cursor at the end of paragraph, no slection, no paragraph related change
            // FIX ME - if changes in PP are not applied for text lets disable it or let select word if nothing is selected
            return;
        if (CPos >= itemText.length() && itemTextSaxed.isEmpty() && action == PARAMFULL && m_Doc->appMode == modeEdit)
        {
            //case when cursor is after last character without selection and nothing was and will be done
            //changes will be ignored
            lastUndoAction = PageItem::NOACTION;
            restoreTextSelection(oldSelStart, oldSelLen);
            return;
        }
        if (action == PARAMFULL && m_Doc->appMode == modeEdit)
        {
            //action is for paragraph where cursor is
            ExpandParSel();
            action = PARAMSEL;
        }
        SimpleState* ss;


Once 0009865 will be commit I upload whole patch for fix this issue.

jghali

2011-03-22 12:48

administrator   ~0025892

Last edited: 2011-03-22 12:58

Generally the ExpandParSel method should be avoided. You should notice its use has been commented out in almost all places. If updateUndo() needs to now about paragraph start or paragraph end position, StoryText already has methods such as startOfParagraph(), endOfParagraph(), previousParagraph() or nextParagaph() which allows to detect the beginning/end of a paragraph without needing to create a temporary selection.

And remember : user selection *must* always be restored. As a consequence currently both updateUndo() and getItemSaxed() must restore user selection as current code does not ensure that updateUndo() or getItemSaxed() are called by methods which will restore it. Previous code caused multiple workflow interruption caused by spurious text selection creation, needing user to deselect text before being able to type again, and things like that.

jghali

2011-03-26 14:12

administrator   ~0025932

Works now after 0009865 fix

Issue History

Date Modified Username Field Change
2011-03-22 11:13 cezaryece New Issue
2011-03-22 11:16 cezaryece Note Added: 0025886
2011-03-22 12:48 jghali Note Added: 0025892
2011-03-22 12:58 jghali Note Edited: 0025892
2011-03-26 14:12 jghali Note Added: 0025932
2011-03-26 14:12 jghali Status new => resolved
2011-03-26 14:12 jghali Fixed in Version => 1.4.0svn
2011-03-26 14:12 jghali Resolution open => fixed
2011-03-26 14:12 jghali Assigned To => jghali
2011-03-27 14:34 cbradney Status resolved => closed