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
(cherry picked from commit 96be992ccdc7a9aaf02672234be7bd341be105e7)

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 fc0cc0d..2ac4b81 100644
--- a/tools/designer/src/components/formeditor/formwindow.cpp
+++ b/tools/designer/src/components/formeditor/formwindow.cpp
@@ -1427,26 +1427,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;
@@ -1479,57 +1576,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 c8e63cf..debc5f6 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 bc9f85b..8e99a6f 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 65e4212..9f696f6 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 6c6c2ae..910aa5b 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 0c883ab..6ca0678 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();