MOS
/
OpenBoard
5
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #336 from multun/movingitem-ub

Browse Source 
fix use after free of movingItem
preferencesAboutTextFull
kaamui 5 years ago committed by GitHub
parent 600f1bccef fb25c17e4e
commit 97669ef5ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 105 additions and 74 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 149
      src/board/UBBoardView.cpp
  2. 30
      src/board/UBBoardView.h

149
src/board/UBBoardView.cpp
Unescape View File

@ -95,6 +95,7 @@ UBBoardView::UBBoardView (UBBoardController* pController, QWidget* pParent, bool
, mLongPressInterval(1000) , mLongPressInterval(1000)
, mIsDragInProgress(false) , mIsDragInProgress(false)
, mMultipleSelectionIsEnabled(false) , mMultipleSelectionIsEnabled(false)
, _movingItem(nullptr)
, bIsControl(isControl) , bIsControl(isControl)
, bIsDesktop(isDesktop) , bIsDesktop(isDesktop)
{ {
@ -119,6 +120,7 @@ UBBoardView::UBBoardView (UBBoardController* pController, int pStartLayer, int p
, mLongPressInterval(1000) , mLongPressInterval(1000)
, mIsDragInProgress(false) , mIsDragInProgress(false)
, mMultipleSelectionIsEnabled(false) , mMultipleSelectionIsEnabled(false)
, _movingItem(nullptr)
, bIsControl(isControl) , bIsControl(isControl)
, bIsDesktop(isDesktop) , bIsDesktop(isDesktop)
{ {
@ -178,7 +180,7 @@ void UBBoardView::init ()
unsetCursor(); unsetCursor();
movingItem = NULL; setMovingItem(NULL);
mWidgetMoved = false; mWidgetMoved = false;
} }
@ -482,7 +484,7 @@ void UBBoardView::handleItemsSelection(QGraphicsItem *item)
if (item) if (item)
{ {
// item has group as first parent - it is any item or UBGraphicsStrokesGroup. // item has group as first parent - it is any item or UBGraphicsStrokesGroup.
if(item->parentItem() && UBGraphicsGroupContainerItem::Type == movingItem->parentItem()->type()) if(item->parentItem() && UBGraphicsGroupContainerItem::Type == getMovingItem()->parentItem()->type())
return; return;
// delegate buttons shouldn't selected // delegate buttons shouldn't selected
@ -578,7 +580,7 @@ Here we determines cases when items should to get mouse press event at pressing
case UBGraphicsGroupContainerItem::Type: case UBGraphicsGroupContainerItem::Type:
if(currentTool == UBStylusTool::Play) if(currentTool == UBStylusTool::Play)
{ {
movingItem = NULL; setMovingItem(NULL);
return true; return true;
} }
return false; return false;
@ -648,10 +650,10 @@ bool UBBoardView::itemShouldBeMoved(QGraphicsItem *item)
if (!(mMouseButtonIsPressed || mTabletStylusIsPressed)) if (!(mMouseButtonIsPressed || mTabletStylusIsPressed))
return false; return false;
if (movingItem->data(UBGraphicsItemData::ItemLocked).toBool()) if (getMovingItem()->data(UBGraphicsItemData::ItemLocked).toBool())
return false; return false;
if (movingItem->parentItem() && UBGraphicsGroupContainerItem::Type == movingItem->parentItem()->type() && !movingItem->isSelected() && movingItem->parentItem()->isSelected()) if (getMovingItem()->parentItem() && UBGraphicsGroupContainerItem::Type == getMovingItem()->parentItem()->type() && !getMovingItem()->isSelected() && getMovingItem()->parentItem()->isSelected())
return false; return false;
UBStylusTool::Enum currentTool = (UBStylusTool::Enum)UBDrawingController::drawingController()->stylusTool(); UBStylusTool::Enum currentTool = (UBStylusTool::Enum)UBDrawingController::drawingController()->stylusTool();
@ -759,13 +761,13 @@ void UBBoardView::handleItemMousePress(QMouseEvent *event)
// Determining item who will take mouse press event // Determining item who will take mouse press event
//all other items will be deselected and if all item will be deselected, then //all other items will be deselected and if all item will be deselected, then
// wrong item can catch mouse press. because selected items placed on the top // wrong item can catch mouse press. because selected items placed on the top
movingItem = determineItemToPress(movingItem); setMovingItem(determineItemToPress(getMovingItem()));
handleItemsSelection(movingItem); handleItemsSelection(getMovingItem());
if (isMultipleSelectionEnabled()) if (isMultipleSelectionEnabled())
return; return;
if (itemShouldReceiveMousePressEvent(movingItem)){ if (itemShouldReceiveMousePressEvent(getMovingItem())){
QGraphicsView::mousePressEvent (event); QGraphicsView::mousePressEvent (event);
QGraphicsItem* item = determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), transform())); QGraphicsItem* item = determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), transform()));
@ -780,17 +782,17 @@ void UBBoardView::handleItemMousePress(QMouseEvent *event)
if ((*it)->pos().x() < 0 || (*it)->pos().y() < 0) if ((*it)->pos().x() < 0 || (*it)->pos().y() < 0)
(*it)->setPos(0,item->boundingRect().size().height()); (*it)->setPos(0,item->boundingRect().size().height());
} }
movingItem = item; setMovingItem(item);
} }
else else
{ {
if (movingItem) if (getMovingItem())
{ {
UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(movingItem); UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(getMovingItem());
if (graphicsItem) if (graphicsItem)
graphicsItem->Delegate()->startUndoStep(); graphicsItem->Delegate()->startUndoStep();
movingItem->clearFocus(); getMovingItem()->clearFocus();
} }
if (suspendedMousePressEvent) if (suspendedMousePressEvent)
@ -799,7 +801,7 @@ void UBBoardView::handleItemMousePress(QMouseEvent *event)
suspendedMousePressEvent = NULL; suspendedMousePressEvent = NULL;
} }
if (itemShouldReceiveSuspendedMousePressEvent(movingItem)) if (itemShouldReceiveSuspendedMousePressEvent(getMovingItem()))
{ {
suspendedMousePressEvent = new QMouseEvent(event->type(), event->pos(), event->button(), event->buttons(), event->modifiers()); suspendedMousePressEvent = new QMouseEvent(event->type(), event->pos(), event->button(), event->buttons(), event->modifiers());
} }
@ -809,14 +811,14 @@ void UBBoardView::handleItemMousePress(QMouseEvent *event)
void UBBoardView::handleItemMouseMove(QMouseEvent *event) void UBBoardView::handleItemMouseMove(QMouseEvent *event)
{ {
// determine item to move (maybee we need to move group of item or his parent. // determine item to move (maybee we need to move group of item or his parent.
movingItem = determineItemToMove(movingItem); setMovingItem(determineItemToMove(getMovingItem()));
// items should be moved not every mouse move. // items should be moved not every mouse move.
if (movingItem && itemShouldBeMoved(movingItem) && (mMouseButtonIsPressed || mTabletStylusIsPressed)) if (getMovingItem() && itemShouldBeMoved(getMovingItem()) && (mMouseButtonIsPressed || mTabletStylusIsPressed))
{ {
QPointF scenePos = mapToScene(event->pos()); QPointF scenePos = mapToScene(event->pos());
QPointF newPos = movingItem->pos() + scenePos - mLastPressedMousePos; QPointF newPos = getMovingItem()->pos() + scenePos - mLastPressedMousePos;
movingItem->setPos(newPos); getMovingItem()->setPos(newPos);
mLastPressedMousePos = scenePos; mLastPressedMousePos = scenePos;
mWidgetMoved = true; mWidgetMoved = true;
event->accept(); event->accept();
@ -826,21 +828,21 @@ void UBBoardView::handleItemMouseMove(QMouseEvent *event)
QPointF posBeforeMove; QPointF posBeforeMove;
QPointF posAfterMove; QPointF posAfterMove;
if (movingItem) if (getMovingItem())
posBeforeMove = movingItem->pos(); posBeforeMove = getMovingItem()->pos();
QGraphicsView::mouseMoveEvent (event); QGraphicsView::mouseMoveEvent (event);
if (movingItem) if (getMovingItem())
posAfterMove = movingItem->pos(); posAfterMove = getMovingItem()->pos();
mWidgetMoved = ((posAfterMove-posBeforeMove).manhattanLength() != 0); mWidgetMoved = ((posAfterMove-posBeforeMove).manhattanLength() != 0);
// a cludge for terminate moving of w3c widgets. // a cludge for terminate moving of w3c widgets.
// in some cases w3c widgets catches mouse move and doesn't sends that events to web page, // in some cases w3c widgets catches mouse move and doesn't sends that events to web page,
// at simple - in google map widget - mouse move events doesn't comes to web page from rectangle of wearch bar on bottom right corner of widget. // at simple - in google map widget - mouse move events doesn't comes to web page from rectangle of wearch bar on bottom right corner of widget.
if (movingItem && mWidgetMoved && UBGraphicsW3CWidgetItem::Type == movingItem->type()) if (getMovingItem() && mWidgetMoved && UBGraphicsW3CWidgetItem::Type == getMovingItem()->type())
movingItem->setPos(posBeforeMove); getMovingItem()->setPos(posBeforeMove);
} }
} }
@ -992,7 +994,7 @@ void UBBoardView::mousePressEvent (QMouseEvent *event)
} }
mMouseDownPos = event->pos (); mMouseDownPos = event->pos ();
movingItem = scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform()); setMovingItem(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform()));
if (event->button () == Qt::LeftButton && isInteractive()) if (event->button () == Qt::LeftButton && isInteractive())
{ {
@ -1024,11 +1026,11 @@ void UBBoardView::mousePressEvent (QMouseEvent *event)
return; return;
} }
if (scene()->backgroundObject() == movingItem) if (scene()->backgroundObject() == getMovingItem())
movingItem = NULL; setMovingItem(NULL);
connect(&mLongPressTimer, SIGNAL(timeout()), this, SLOT(longPressEvent())); connect(&mLongPressTimer, SIGNAL(timeout()), this, SLOT(longPressEvent()));
if (!movingItem && !mController->cacheIsVisible()) if (!getMovingItem() && !mController->cacheIsVisible())
mLongPressTimer.start(); mLongPressTimer.start();
handleItemMousePress(event); handleItemMousePress(event);
@ -1143,7 +1145,7 @@ void UBBoardView::mouseMoveEvent (QMouseEvent *event)
bool rubberMove = (currentTool != (UBStylusTool::Play)) bool rubberMove = (currentTool != (UBStylusTool::Play))
&& (mMouseButtonIsPressed || mTabletStylusIsPressed) && (mMouseButtonIsPressed || mTabletStylusIsPressed)
&& !movingItem; && !getMovingItem();
if (rubberMove) { if (rubberMove) {
QRect bandRect(mMouseDownPos, event->pos()); QRect bandRect(mMouseDownPos, event->pos());
@ -1229,6 +1231,11 @@ void UBBoardView::mouseMoveEvent (QMouseEvent *event)
} }
void UBBoardView::movingItemDestroyed(QObject*)
{
setMovingItem(nullptr);
}
void UBBoardView::mouseReleaseEvent (QMouseEvent *event) void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
{ {
UBStylusTool::Enum currentTool = (UBStylusTool::Enum)UBDrawingController::drawingController ()->stylusTool (); UBStylusTool::Enum currentTool = (UBStylusTool::Enum)UBDrawingController::drawingController ()->stylusTool ();
@ -1245,60 +1252,60 @@ void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
return; return;
} }
UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(movingItem); UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(getMovingItem());
if (graphicsItem) if (graphicsItem)
graphicsItem->Delegate()->commitUndoStep(); graphicsItem->Delegate()->commitUndoStep();
bool bReleaseIsNeed = true; bool bReleaseIsNeed = true;
if (movingItem != determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform()))) if (getMovingItem() != determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform())))
{ {
movingItem = NULL; setMovingItem(nullptr);
bReleaseIsNeed = false; bReleaseIsNeed = false;
} }
if (mWidgetMoved) if (mWidgetMoved)
{ {
mWidgetMoved = false; mWidgetMoved = false;
movingItem = NULL; setMovingItem(nullptr);
} }
else else
if (movingItem && (!isCppTool(movingItem) || UBGraphicsCurtainItem::Type == movingItem->type())) if (getMovingItem() && (!isCppTool(getMovingItem()) || UBGraphicsCurtainItem::Type == getMovingItem()->type()))
{ {
if (suspendedMousePressEvent) if (suspendedMousePressEvent)
{ {
QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop
movingItem = NULL; setMovingItem(NULL);
delete suspendedMousePressEvent; delete suspendedMousePressEvent;
suspendedMousePressEvent = NULL; suspendedMousePressEvent = NULL;
bReleaseIsNeed = true; bReleaseIsNeed = true;
} }
else else
{ {
if (isUBItem(movingItem) && if (isUBItem(getMovingItem()) &&
DelegateButton::Type != movingItem->type() && DelegateButton::Type != getMovingItem()->type() &&
UBGraphicsDelegateFrame::Type != movingItem->type() && UBGraphicsDelegateFrame::Type != getMovingItem()->type() &&
UBGraphicsCache::Type != movingItem->type() && UBGraphicsCache::Type != getMovingItem()->type() &&
QGraphicsWebView::Type != movingItem->type() && // for W3C widgets as Tools. QGraphicsWebView::Type != getMovingItem()->type() && // for W3C widgets as Tools.
!(!isMultipleSelectionEnabled() && movingItem->parentItem() && UBGraphicsWidgetItem::Type == movingItem->type() && UBGraphicsGroupContainerItem::Type == movingItem->parentItem()->type())) !(!isMultipleSelectionEnabled() && getMovingItem()->parentItem() && UBGraphicsWidgetItem::Type == getMovingItem()->type() && UBGraphicsGroupContainerItem::Type == getMovingItem()->parentItem()->type()))
{ {
bReleaseIsNeed = false; bReleaseIsNeed = false;
if (movingItem->isSelected() && isMultipleSelectionEnabled()) if (getMovingItem()->isSelected() && isMultipleSelectionEnabled())
movingItem->setSelected(false); getMovingItem()->setSelected(false);
else else
if (movingItem->parentItem() && movingItem->parentItem()->isSelected() && isMultipleSelectionEnabled()) if (getMovingItem()->parentItem() && getMovingItem()->parentItem()->isSelected() && isMultipleSelectionEnabled())
movingItem->parentItem()->setSelected(false); getMovingItem()->parentItem()->setSelected(false);
else else
{ {
if (movingItem->isSelected()) if (getMovingItem()->isSelected())
bReleaseIsNeed = true; bReleaseIsNeed = true;
UBGraphicsTextItem* textItem = dynamic_cast<UBGraphicsTextItem*>(movingItem); UBGraphicsTextItem* textItem = dynamic_cast<UBGraphicsTextItem*>(getMovingItem());
UBGraphicsMediaItem* movieItem = dynamic_cast<UBGraphicsMediaItem*>(movingItem); UBGraphicsMediaItem* movieItem = dynamic_cast<UBGraphicsMediaItem*>(getMovingItem());
if(textItem) if(textItem)
textItem->setSelected(true); textItem->setSelected(true);
else if(movieItem) else if(movieItem)
movieItem->setSelected(true); movieItem->setSelected(true);
else else
movingItem->setSelected(true); getMovingItem()->setSelected(true);
} }
} }
@ -1315,20 +1322,20 @@ void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
else if (currentTool == UBStylusTool::Text) else if (currentTool == UBStylusTool::Text)
{ {
bool bReleaseIsNeed = true; bool bReleaseIsNeed = true;
if (movingItem != determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform()))) if (getMovingItem() != determineItemToPress(scene()->itemAt(this->mapToScene(event->localPos().toPoint()), QTransform())))
{ {
movingItem = NULL; setMovingItem(NULL);
bReleaseIsNeed = false; bReleaseIsNeed = false;
} }
UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(movingItem); UBGraphicsItem *graphicsItem = dynamic_cast<UBGraphicsItem*>(getMovingItem());
if (graphicsItem) if (graphicsItem)
graphicsItem->Delegate()->commitUndoStep(); graphicsItem->Delegate()->commitUndoStep();
if (mWidgetMoved) if (mWidgetMoved)
{ {
mWidgetMoved = false; mWidgetMoved = false;
movingItem = NULL; setMovingItem(NULL);
if (scene () && mRubberBand && mIsCreatingTextZone) { if (scene () && mRubberBand && mIsCreatingTextZone) {
QRect rubberRect = mRubberBand->geometry (); QRect rubberRect = mRubberBand->geometry ();
@ -1343,37 +1350,37 @@ void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
textItem->setFocus(); textItem->setFocus();
} }
} }
else if (movingItem && (!isCppTool(movingItem) || UBGraphicsCurtainItem::Type == movingItem->type())) else if (getMovingItem() && (!isCppTool(getMovingItem()) || UBGraphicsCurtainItem::Type == getMovingItem()->type()))
{ {
if (suspendedMousePressEvent) if (suspendedMousePressEvent)
{ {
QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop
movingItem = NULL; setMovingItem(NULL);
delete suspendedMousePressEvent; delete suspendedMousePressEvent;
suspendedMousePressEvent = NULL; suspendedMousePressEvent = NULL;
bReleaseIsNeed = true; bReleaseIsNeed = true;
} }
else{ else{
if (isUBItem(movingItem) && if (isUBItem(getMovingItem()) &&
DelegateButton::Type != movingItem->type() && DelegateButton::Type != getMovingItem()->type() &&
QGraphicsSvgItem::Type != movingItem->type() && QGraphicsSvgItem::Type != getMovingItem()->type() &&
UBGraphicsDelegateFrame::Type != movingItem->type() && UBGraphicsDelegateFrame::Type != getMovingItem()->type() &&
UBGraphicsCache::Type != movingItem->type() && UBGraphicsCache::Type != getMovingItem()->type() &&
QGraphicsWebView::Type != movingItem->type() && // for W3C widgets as Tools. QGraphicsWebView::Type != getMovingItem()->type() && // for W3C widgets as Tools.
!(!isMultipleSelectionEnabled() && movingItem->parentItem() && UBGraphicsWidgetItem::Type == movingItem->type() && UBGraphicsGroupContainerItem::Type == movingItem->parentItem()->type())) !(!isMultipleSelectionEnabled() && getMovingItem()->parentItem() && UBGraphicsWidgetItem::Type == getMovingItem()->type() && UBGraphicsGroupContainerItem::Type == getMovingItem()->parentItem()->type()))
{ {
bReleaseIsNeed = false; bReleaseIsNeed = false;
if (movingItem->isSelected() && isMultipleSelectionEnabled()) if (getMovingItem()->isSelected() && isMultipleSelectionEnabled())
movingItem->setSelected(false); getMovingItem()->setSelected(false);
else else
if (movingItem->parentItem() && movingItem->parentItem()->isSelected() && isMultipleSelectionEnabled()) if (getMovingItem()->parentItem() && getMovingItem()->parentItem()->isSelected() && isMultipleSelectionEnabled())
movingItem->parentItem()->setSelected(false); getMovingItem()->parentItem()->setSelected(false);
else else
{ {
if (movingItem->isSelected()) if (getMovingItem()->isSelected())
bReleaseIsNeed = true; bReleaseIsNeed = true;
movingItem->setSelected(true); getMovingItem()->setSelected(true);
} }
} }
@ -1394,14 +1401,14 @@ void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
} }
if (mWidgetMoved) { if (mWidgetMoved) {
movingItem->setSelected(false); getMovingItem()->setSelected(false);
movingItem = NULL; setMovingItem(NULL);
mWidgetMoved = false; mWidgetMoved = false;
} }
else { else {
if (suspendedMousePressEvent) { if (suspendedMousePressEvent) {
QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop QGraphicsView::mousePressEvent(suspendedMousePressEvent); // suspendedMousePressEvent is deleted by old Qt event loop
movingItem = NULL; setMovingItem(NULL);
delete suspendedMousePressEvent; delete suspendedMousePressEvent;
suspendedMousePressEvent = NULL; suspendedMousePressEvent = NULL;
} }
@ -1453,7 +1460,7 @@ void UBBoardView::mouseReleaseEvent (QMouseEvent *event)
mMouseButtonIsPressed = false; mMouseButtonIsPressed = false;
mPendingStylusReleaseEvent = false; mPendingStylusReleaseEvent = false;
mTabletStylusIsPressed = false; mTabletStylusIsPressed = false;
movingItem = NULL; setMovingItem(NULL);
mLongPressTimer.stop(); mLongPressTimer.stop();
emit mouseReleased(); emit mouseReleased();

30
src/board/UBBoardView.h
Unescape View File

@ -156,7 +156,32 @@ private:
bool mWidgetMoved; bool mWidgetMoved;
QPointF mLastPressedMousePos; QPointF mLastPressedMousePos;
QGraphicsItem *movingItem;
/* when an item is moved around, the tracking must stop if the object is deleted */
QGraphicsItem *_movingItem;
QGraphicsItem *getMovingItem() {
return _movingItem;
}
void setMovingItem(QGraphicsItem *item) {
// if the current moving item is a qobject, it MUST be disconnected
if (_movingItem) {
QObject *moving_item_obj = dynamic_cast<QObject*>(_movingItem);
if (moving_item_obj != nullptr)
disconnect(moving_item_obj, &QObject::destroyed, this, &UBBoardView::movingItemDestroyed);
}
// attach the new moving item if relevant
if (item) {
QObject *item_obj = dynamic_cast<QObject*>(item);
if (item_obj != nullptr)
connect(item_obj, &QObject::destroyed, this, &UBBoardView::movingItemDestroyed);
}
_movingItem = item;
}
QMouseEvent *suspendedMousePressEvent; QMouseEvent *suspendedMousePressEvent;
bool moveRubberBand; bool moveRubberBand;
@ -177,11 +202,10 @@ private:
static bool hasSelectedParents(QGraphicsItem * item); static bool hasSelectedParents(QGraphicsItem * item);
private slots: private slots:
void settingChanged(QVariant newValue); void settingChanged(QVariant newValue);
void movingItemDestroyed(QObject* item = nullptr);
public slots: public slots:
void virtualKeyboardActivated(bool b); void virtualKeyboardActivated(bool b);
void longPressEvent(); void longPressEvent();

Write Preview
Loading…
Cancel
Save