Designer:Fix undo crash caused by special handling of key move.

Remove special handling and storing of commands. Customize
SetPropertyCommand such that it is able to perform a transformation
of the new value., thus, the same (relative) distance can be used
on all widgets and the normal command merging mechanism can be used
to merge subsequent moves correctly.
Task-number: QTBUG-8213

6 files changed, 177 insertions, 90 deletions

diff --git a/tools/designer/src/components/formeditor/formwindow.cpp b/tools/designer/src/components/formeditor/formwindow.cpp
index 3a7cd83..8b8351d 100644
--- a/tools/designer/src/components/formeditor/formwindow.cpp
+++ b/tools/designer/src/components/formeditor/formwindow.cpp
@@ -1428,26 +1428,123 @@ int FormWindow::calcValue(int val, bool forward, bool snap, int snapOffset) cons
     return (forward ? val + 1 : val - 1);
 }
 
-QRect FormWindow::applyValue(const QRect &rect, int val, int key, bool size) const
+// ArrowKeyOperation: Stores a keyboard move or resize (Shift pressed)
+// operation.
+struct ArrowKeyOperation {
+    ArrowKeyOperation() : resize(false), distance(0), arrowKey(Qt::Key_Left) {}
+
+    QRect apply(const QRect &in) const;
+
+    bool resize; // Resize: Shift-Key->drag bottom/right corner, else just move
+    int distance;
+    int arrowKey;
+};
+
+QRect ArrowKeyOperation::apply(const QRect &rect) const
 {
     QRect r = rect;
-    if (size) {
-        if (key == Qt::Key_Left || key == Qt::Key_Right)
-            r.setWidth(val);
+    if (resize) {
+        if (arrowKey == Qt::Key_Left || arrowKey == Qt::Key_Right)
+            r.setWidth(r.width() + distance);
         else
-            r.setHeight(val);
+            r.setHeight(r.height() + distance);
     } else {
-        if (key == Qt::Key_Left || key == Qt::Key_Right)
-            r.moveLeft(val);
+        if (arrowKey == Qt::Key_Left || arrowKey == Qt::Key_Right)
+            r.moveLeft(r.x() + distance);
         else
-            r.moveTop(val);
+            r.moveTop(r.y() + distance);
     }
     return r;
 }
 
+QDebug operator<<(QDebug in, const ArrowKeyOperation &op)
+{
+    in.nospace() << "Resize=" << op.resize << " dist=" << op.distance << " Key=" << op.arrowKey << ' ';
+    return in;
+}
+
+} // namespace qdesigner_internal
+
+Q_DECLARE_METATYPE(qdesigner_internal::ArrowKeyOperation)
+
+namespace qdesigner_internal {
+
+// ArrowKeyPropertyHelper: Applies a struct ArrowKeyOperation
+// (stored as new value) to a list of widgets using to calculate the
+// changed geometry of the widget in setValue(). Thus, the 'newValue'
+// of the property command is the relative move distance, which is the same
+// for all widgets (although resulting in different geometries for the widgets).
+// The command merging can then work as it would when applying the same text
+// to all QLabels.
+
+class ArrowKeyPropertyHelper : public PropertyHelper {
+public:
+    ArrowKeyPropertyHelper(QObject* o, SpecialProperty sp,
+                       QDesignerPropertySheetExtension *s, int i) :
+                       PropertyHelper(o, sp, s, i) {}
+
+    virtual Value setValue(QDesignerFormWindowInterface *fw, const QVariant &value, bool changed, unsigned subPropertyMask);
+};
+
+PropertyHelper::Value ArrowKeyPropertyHelper::setValue(QDesignerFormWindowInterface *fw, const QVariant &value, bool changed, unsigned subPropertyMask)
+{
+    // Apply operation to obtain the new geometry value.
+    QWidget *w = qobject_cast<QWidget*>(object());
+    const ArrowKeyOperation operation = qvariant_cast<ArrowKeyOperation>(value);
+    const QRect newGeom = operation.apply(w->geometry());
+    return PropertyHelper::setValue(fw, QVariant(newGeom), changed, subPropertyMask);
+}
+
+// ArrowKeyPropertyCommand: Helper factory overwritten to create
+// ArrowKeyPropertyHelper and a merge operation that merges values of
+// the same direction.
+class ArrowKeyPropertyCommand: public SetPropertyCommand {
+public:
+    explicit ArrowKeyPropertyCommand(QDesignerFormWindowInterface *fw,
+                                     QUndoCommand *p = 0);
+
+    void init(QWidgetList &l, const ArrowKeyOperation &op);
+
+protected:
+    virtual PropertyHelper *createPropertyHelper(QObject *o, SpecialProperty sp,
+                                                 QDesignerPropertySheetExtension *s, int i) const
+        { return new ArrowKeyPropertyHelper(o, sp, s, i); }
+    virtual QVariant mergeValue(const QVariant &newValue);
+};
+
+ArrowKeyPropertyCommand::ArrowKeyPropertyCommand(QDesignerFormWindowInterface *fw,
+                                                 QUndoCommand *p) :
+    SetPropertyCommand(fw, p)
+{
+    static const int mid = qRegisterMetaType<qdesigner_internal::ArrowKeyOperation>();
+    Q_UNUSED(mid)
+}
+
+void ArrowKeyPropertyCommand::init(QWidgetList &l, const ArrowKeyOperation &op)
+{
+    QObjectList ol;
+    foreach(QWidget *w, l)
+        ol.push_back(w);
+    SetPropertyCommand::init(ol, QLatin1String("geometry"), qVariantFromValue(op));
+
+    setText(op.resize ? FormWindow::tr("Key Resize") : FormWindow::tr("Key Move"));
+}
+
+QVariant ArrowKeyPropertyCommand::mergeValue(const QVariant &newMergeValue)
+{
+    // Merge move operations of the same arrow key
+    if (!qVariantCanConvert<ArrowKeyOperation>(newMergeValue))
+        return QVariant();
+    ArrowKeyOperation mergedOperation = qvariant_cast<ArrowKeyOperation>(newValue());
+    const ArrowKeyOperation newMergeOperation = qvariant_cast<ArrowKeyOperation>(newMergeValue);
+    if (mergedOperation.resize != newMergeOperation.resize || mergedOperation.arrowKey != newMergeOperation.arrowKey)
+        return QVariant();
+    mergedOperation.distance += newMergeOperation.distance;
+    return qVariantFromValue(mergedOperation);
+}
+
 void FormWindow::handleArrowKeyEvent(int key, Qt::KeyboardModifiers modifiers)
 {
-    bool startMacro = false;
     const QDesignerFormWindowCursorInterface *c = cursor();
     if (!c->hasSelection())
         return;
@@ -1480,57 +1577,14 @@ void FormWindow::handleArrowKeyEvent(int key, Qt::KeyboardModifiers modifiers)
 
     const int newValue = calcValue(oldValue, forward, snap, snapPoint);
 
-    const int offset = newValue - oldValue;
+    ArrowKeyOperation operation;
+    operation.resize = modifiers & Qt::ShiftModifier;
+    operation.distance = newValue - oldValue;
+    operation.arrowKey = key;
 
-    const int selCount = selection.count();
-    // check if selection is the same as last time
-    if (selCount != m_moveSelection.count() ||
-        m_lastUndoIndex != m_commandHistory->index()) {
-        m_moveSelection.clear();
-        startMacro = true;
-    } else {
-        for (int index = 0; index < selCount; ++index) {
-            if (m_moveSelection[index]->object() != selection.at(index)) {
-                m_moveSelection.clear();
-                startMacro = true;
-                break;
-            }
-        }
-    }
-
-    if (startMacro)
-        beginCommand(tr("Key Move"));
-
-    for (int index = 0; index < selCount; ++index) {
-        QWidget *w = selection.at(index);
-        const QRect oldGeom = w->geometry();
-        const QRect geom = applyValue(oldGeom, getValue(oldGeom, key, size) + offset, key, size);
-
-        SetPropertyCommand *cmd = 0;
-
-        if (m_moveSelection.count() > index)
-            cmd = m_moveSelection[index];
-
-        if (!cmd) {
-            cmd = new SetPropertyCommand(this);
-            cmd->init(w, QLatin1String("geometry"), geom);
-            cmd->setText(tr("Key Move"));
-            m_commandHistory->push(cmd);
-
-            if (m_moveSelection.count() > index)
-                m_moveSelection.replace(index, cmd);
-            else
-                m_moveSelection.append(cmd);
-        } else {
-            cmd->setNewValue(geom);
-            cmd->redo();
-        }
-    }
-
-    if (startMacro) {
-        endCommand();
-        m_lastUndoIndex = m_commandHistory->index();
-    }
+    ArrowKeyPropertyCommand *cmd = new ArrowKeyPropertyCommand(this);
+    cmd->init(selection, operation);
+    m_commandHistory->push(cmd);
 }
 
 bool FormWindow::handleKeyReleaseEvent(QWidget *, QWidget *, QKeyEvent *e)
diff --git a/tools/designer/src/components/formeditor/formwindow.h b/tools/designer/src/components/formeditor/formwindow.h
index 3eee476..1b2916c 100644
--- a/tools/designer/src/components/formeditor/formwindow.h
+++ b/tools/designer/src/components/formeditor/formwindow.h
@@ -263,7 +263,6 @@ private:
 
     int getValue(const QRect &rect, int key, bool size) const;
     int calcValue(int val, bool forward, bool snap, int snapOffset) const;
-    QRect applyValue(const QRect &rect, int val, int key, bool size) const;
     void handleClickSelection(QWidget *managedWidget, unsigned mouseFlags);
 
     bool frameNeeded(QWidget *w) const;
@@ -368,8 +367,6 @@ private:
     QString m_exportMacro;
     QStringList m_includeHints;
 
-    QList<SetPropertyCommand*> m_moveSelection;
-    int m_lastUndoIndex;
     QPoint m_contextMenuPosition;
 
 private:
diff --git a/tools/designer/src/lib/shared/qdesigner_formwindowcommand.cpp b/tools/designer/src/lib/shared/qdesigner_formwindowcommand.cpp
index 8c55d26..490373e 100644
--- a/tools/designer/src/lib/shared/qdesigner_formwindowcommand.cpp
+++ b/tools/designer/src/lib/shared/qdesigner_formwindowcommand.cpp
@@ -62,8 +62,10 @@ QT_BEGIN_NAMESPACE
 namespace qdesigner_internal {
 
 // ---- QDesignerFormWindowCommand ----
-QDesignerFormWindowCommand::QDesignerFormWindowCommand(const QString &description, QDesignerFormWindowInterface *formWindow)
-    : QUndoCommand(description),
+QDesignerFormWindowCommand::QDesignerFormWindowCommand(const QString &description,
+                                                       QDesignerFormWindowInterface *formWindow,
+                                                       QUndoCommand *parent)
+    : QUndoCommand(description, parent),
       m_formWindow(formWindow)
 {
 }
diff --git a/tools/designer/src/lib/shared/qdesigner_formwindowcommand_p.h b/tools/designer/src/lib/shared/qdesigner_formwindowcommand_p.h
index d8cd018..d73d70c 100644
--- a/tools/designer/src/lib/shared/qdesigner_formwindowcommand_p.h
+++ b/tools/designer/src/lib/shared/qdesigner_formwindowcommand_p.h
@@ -70,7 +70,9 @@ class QDESIGNER_SHARED_EXPORT QDesignerFormWindowCommand: public QUndoCommand
 {
 
 public:
-    QDesignerFormWindowCommand(const QString &description, QDesignerFormWindowInterface *formWindow);
+    QDesignerFormWindowCommand(const QString &description,
+                               QDesignerFormWindowInterface *formWindow,
+                               QUndoCommand *parent = 0);
 
     virtual void undo();
     virtual void redo();
diff --git a/tools/designer/src/lib/shared/qdesigner_propertycommand.cpp b/tools/designer/src/lib/shared/qdesigner_propertycommand.cpp
index 822c14b..6cc054c 100644
--- a/tools/designer/src/lib/shared/qdesigner_propertycommand.cpp
+++ b/tools/designer/src/lib/shared/qdesigner_propertycommand.cpp
@@ -926,8 +926,9 @@ bool PropertyListCommand::PropertyDescription::equals(const PropertyDescription
 
 
 // ---- PropertyListCommand
-PropertyListCommand::PropertyListCommand(QDesignerFormWindowInterface *formWindow) :
-    QDesignerFormWindowCommand(QString(), formWindow)
+PropertyListCommand::PropertyListCommand(QDesignerFormWindowInterface *formWindow,
+                                         QUndoCommand *parent) :
+    QDesignerFormWindowCommand(QString(), formWindow, parent)
 {
 }
 
@@ -966,10 +967,17 @@ bool PropertyListCommand::add(QObject *object, const QString &propertyName)
         if (!match || m_propertyDescription.m_specialProperty == SP_ObjectName)
             return false;
     }
-    m_propertyHelperList.push_back(PropertyHelper(object, m_propertyDescription.m_specialProperty, sheet, index));
+
+    const PropertyHelperPtr ph(createPropertyHelper(object, m_propertyDescription.m_specialProperty, sheet, index));
+    m_propertyHelperList.push_back(ph);
     return true;
 }
 
+PropertyHelper *PropertyListCommand::createPropertyHelper(QObject *object, SpecialProperty sp,
+                                                          QDesignerPropertySheetExtension *sheet, int sheetIndex) const
+{
+    return new PropertyHelper(object, sp, sheet, sheetIndex);
+}
 
 // Init from a list and make sure referenceObject is added first to obtain the right property group
 bool PropertyListCommand::initList(const ObjectList &list, const QString &apropertyName, QObject *referenceObject)
@@ -993,19 +1001,19 @@ bool PropertyListCommand::initList(const ObjectList &list, const QString &aprope
 QObject* PropertyListCommand::object(int index) const
 {
     Q_ASSERT(index < m_propertyHelperList.size());
-    return m_propertyHelperList[index].object();
+    return m_propertyHelperList.at(index)->object();
 }
 
 QVariant PropertyListCommand::oldValue(int index) const
 {
     Q_ASSERT(index < m_propertyHelperList.size());
-    return m_propertyHelperList[index].oldValue();
+    return m_propertyHelperList.at(index)->oldValue();
 }
 
 void PropertyListCommand::setOldValue(const QVariant &oldValue, int index)
 {
     Q_ASSERT(index < m_propertyHelperList.size());
-    m_propertyHelperList[index].setOldValue(oldValue);
+    m_propertyHelperList.at(index)->setOldValue(oldValue);
 }
 // ----- SetValueFunction: Set a new value when applied to a PropertyHelper.
 class SetValueFunction {
@@ -1065,9 +1073,10 @@ template <class PropertyListIterator, class Function>
     bool updatedPropertyEditor = false;
 
     for (PropertyListIterator it = begin; it != end; ++it) {
-        if (QObject* object = it->object()) { // Might have been deleted in the meantime
-            const PropertyHelper::Value newValue = function(*it);
-            updateMask |= it->updateMask();
+        PropertyHelper *ph = it->data();
+        if (QObject* object = ph->object()) { // Might have been deleted in the meantime
+            const PropertyHelper::Value newValue = function( *ph );
+            updateMask |= ph->updateMask();
             // Update property editor if it is the current object
             if (!updatedPropertyEditor && propertyEditor && object == propertyEditor->object()) {
                 propertyEditor->setPropertyValue(propertyName, newValue.first,  newValue.second);
@@ -1084,9 +1093,11 @@ template <class PropertyListIterator, class Function>
 unsigned PropertyListCommand::setValue(QVariant value, bool changed, unsigned subPropertyMask)
 {
     if(debugPropertyCommands)
-        qDebug() << "PropertyListCommand::setValue(" << value <<  changed << subPropertyMask << ')';
+        qDebug() << "PropertyListCommand::setValue(" << value
+                 << changed << subPropertyMask << ')';
     return changePropertyList(formWindow()->core(),
-                              m_propertyDescription.m_propertyName, m_propertyHelperList.begin(), m_propertyHelperList.end(),
+                              m_propertyDescription.m_propertyName,
+                              m_propertyHelperList.begin(), m_propertyHelperList.end(),
                               SetValueFunction(formWindow(), PropertyHelper::Value(value, changed), subPropertyMask));
 }
 
@@ -1146,15 +1157,16 @@ bool PropertyListCommand::canMergeLists(const PropertyHelperList& other) const
     if (m_propertyHelperList.size() !=  other.size())
         return false;
     for (int i = 0; i < m_propertyHelperList.size(); i++) {
-        if (!m_propertyHelperList[i].canMerge(other[i]))
+        if (!m_propertyHelperList.at(i)->canMerge(*other.at(i)))
             return false;
     }
     return true;
 }
 
 // ---- SetPropertyCommand ----
-SetPropertyCommand::SetPropertyCommand(QDesignerFormWindowInterface *formWindow)
-    :  PropertyListCommand(formWindow),
+SetPropertyCommand::SetPropertyCommand(QDesignerFormWindowInterface *formWindow,
+                                       QUndoCommand *parent)
+    :  PropertyListCommand(formWindow, parent),
        m_subPropertyMask(SubPropertyAll)
 {
 }
@@ -1210,7 +1222,7 @@ unsigned SetPropertyCommand::subPropertyMask(const QVariant &newValue, QObject *
 void SetPropertyCommand::setDescription()
 {
     if (propertyHelperList().size() == 1) {
-        setText(QApplication::translate("Command", "Changed '%1' of '%2'").arg(propertyName()).arg(propertyHelperList()[0].object()->objectName()));
+        setText(QApplication::translate("Command", "Changed '%1' of '%2'").arg(propertyName()).arg(propertyHelperList().at(0)->object()->objectName()));
     } else {
         int count = propertyHelperList().size();
         setText(QApplication::translate("Command", "Changed '%1' of %n objects", "", QCoreApplication::UnicodeUTF8, count).arg(propertyName()));
@@ -1231,6 +1243,11 @@ int SetPropertyCommand::id() const
     return 1976;
 }
 
+QVariant SetPropertyCommand::mergeValue(const QVariant &newValue)
+{
+    return newValue;
+}
+
 bool SetPropertyCommand::mergeWith(const QUndoCommand *other)
 {
     if (id() != other->id() || !formWindow()->isDirty())
@@ -1248,7 +1265,10 @@ bool SetPropertyCommand::mergeWith(const QUndoCommand *other)
         !canMergeLists(cmd->propertyHelperList()))
         return false;
 
-    m_newValue = cmd->newValue();
+    const QVariant newValue = mergeValue(cmd->newValue());
+    if (!newValue.isValid())
+        return false;
+    m_newValue = newValue;
     m_subPropertyMask |= cmd->m_subPropertyMask;
     if(debugPropertyCommands)
         qDebug() << "SetPropertyCommand::mergeWith() succeeded " << propertyName();
@@ -1289,7 +1309,7 @@ bool ResetPropertyCommand::init(const ObjectList &list, const QString &aproperty
 void ResetPropertyCommand::setDescription()
 {
     if (propertyHelperList().size() == 1) {
-        setText(QApplication::translate("Command", "Reset '%1' of '%2'").arg(propertyName()).arg(propertyHelperList()[0].object()->objectName()));
+        setText(QApplication::translate("Command", "Reset '%1' of '%2'").arg(propertyName()).arg(propertyHelperList().at(0)->object()->objectName()));
     } else {
         int count = propertyHelperList().size();
         setText(QApplication::translate("Command", "Reset '%1' of %n objects", "", QCoreApplication::UnicodeUTF8, count).arg(propertyName()));
diff --git a/tools/designer/src/lib/shared/qdesigner_propertycommand_p.h b/tools/designer/src/lib/shared/qdesigner_propertycommand_p.h
index f6b7262..75b23ca 100644
--- a/tools/designer/src/lib/shared/qdesigner_propertycommand_p.h
+++ b/tools/designer/src/lib/shared/qdesigner_propertycommand_p.h
@@ -58,6 +58,7 @@
 #include <QtCore/QVariant>
 #include <QtCore/QList>
 #include <QtCore/QPair>
+#include <QtCore/QSharedPointer>
 
 QT_BEGIN_NAMESPACE
 
@@ -77,11 +78,12 @@ enum SpecialProperty {
 //Determine special property
 enum SpecialProperty getSpecialProperty(const QString& propertyName);
 
-
 // A helper class for applying properties to objects.
 // Can be used for Set commands (setValue(), restoreOldValue()) or
 // Reset Commands     restoreDefaultValue(), restoreOldValue()).
-class PropertyHelper {
+//
+class QDESIGNER_SHARED_EXPORT PropertyHelper {
+    Q_DISABLE_COPY(PropertyHelper)
 public:
     // A pair of Value and changed flag
     typedef QPair<QVariant, bool> Value;
@@ -92,11 +94,13 @@ public:
                    SpecialProperty specialProperty,
                    QDesignerPropertySheetExtension *sheet,
                    int index);
+    virtual ~PropertyHelper() {}
 
     QObject *object() const { return m_object; }
     SpecialProperty specialProperty() const { return m_specialProperty; }
-    // set a new value
-    Value setValue(QDesignerFormWindowInterface *fw, const QVariant &value, bool changed, unsigned subPropertyMask);
+    // set a new value. Can be overwritten to perform a transformation (see
+    // handling of Arrow key move in FormWindow class).
+    virtual Value setValue(QDesignerFormWindowInterface *fw, const QVariant &value, bool changed, unsigned subPropertyMask);
 
     // restore old value
     Value restoreOldValue(QDesignerFormWindowInterface *fw);
@@ -147,7 +151,7 @@ class QDESIGNER_SHARED_EXPORT PropertyListCommand : public QDesignerFormWindowCo
 public:
     typedef QList<QObject *> ObjectList;
 
-    explicit PropertyListCommand(QDesignerFormWindowInterface *formWindow);
+    explicit PropertyListCommand(QDesignerFormWindowInterface *formWindow, QUndoCommand *parent = 0);
 
     QObject* object(int index = 0) const;
 
@@ -159,8 +163,8 @@ public:
     virtual void undo();
 
 protected:
-    typedef QList<PropertyHelper> PropertyHelperList;
-
+    typedef QSharedPointer<PropertyHelper> PropertyHelperPtr;
+    typedef QList<PropertyHelperPtr> PropertyHelperList;
 
     // add an object
     bool add(QObject *object, const QString &propertyName);
@@ -204,6 +208,10 @@ protected:
     };
     const PropertyDescription &propertyDescription() const { return  m_propertyDescription; }
 
+protected:
+    virtual PropertyHelper *createPropertyHelper(QObject *o, SpecialProperty sp,
+                                                 QDesignerPropertySheetExtension *sheet, int sheetIndex) const;
+
 private:
     PropertyDescription m_propertyDescription;
     PropertyHelperList m_propertyHelperList;
@@ -215,7 +223,7 @@ class QDESIGNER_SHARED_EXPORT SetPropertyCommand: public PropertyListCommand
 public:
     typedef QList<QObject *> ObjectList;
 
-    explicit SetPropertyCommand(QDesignerFormWindowInterface *formWindow);
+    explicit SetPropertyCommand(QDesignerFormWindowInterface *formWindow, QUndoCommand *parent = 0);
 
     bool init(QObject *object, const QString &propertyName, const QVariant &newValue);
     bool init(const ObjectList &list, const QString &propertyName, const QVariant &newValue,
@@ -232,6 +240,10 @@ public:
     bool mergeWith(const QUndoCommand *other);
 
     virtual void redo();
+
+protected:
+    virtual QVariant mergeValue(const QVariant &newValue);
+
 private:
     unsigned subPropertyMask(const QVariant &newValue, QObject *referenceObject);
     void setDescription();