Fix #591's resize handles: reachable only via Shift/right-click, wrong position

Two bugs reported by @arummler on #591 after merge:

  "It works but to select the text field one has to right click on
  it...I think there are competing handlers or something."
  "the drag elements should be on the border of the box. In the
  moment they appear directly left and right from the text."

Both reproduced headlessly (scripts/qet-gui-dialog.sh) against a fresh
build of current master and root-caused before touching anything.

Selection: DynamicElementTextItem::mousePressEvent() forwards a plain
click (no Shift) straight to parentElement()->mousePressEvent(), by
design and pre-existing -- it's what lets dragging a symbol by its own
label move the whole symbol rather than just the label. That's correct
and untouched here. But it means a plain click leaves the *parent*
selected, not the text, and #591's handles were wired only to the
text's own ItemSelectedHasChanged -- so they were only reachable via
Shift+click or a right-click's context menu (which happens to select
the item under the cursor for its own context menu, unrelated to the
Shift path), neither of which anyone reaches for to resize a text.
Confirmed with screenshots at each step, including that Shift+click
already reached the existing (if misplaced) handles correctly.

Fix: DynamicElementTextItem::refreshResizeHandlesVisibility() shows the
handles when either the text itself or its parent element is selected,
and Element gets an itemChange() override (it had none) that calls it
on each of its own texts when the element's own selection changes. Both
sides driven from itemChange(), Qt's own hook for exactly this and the
same one already used for the text's own selection.

First attempt drove this from paint() instead, since the PR's own
updateResizeHandlesPos() already runs there. That crashed reproducibly
(SIGABRT) on deselecting a text: paint() runs while QGraphicsScene
iterates its item list to draw it, and addResizeHandles()/
removeResizeHandles() mutate that list via QGraphicsScene::addItem()/
removeItem(), which cannot safely happen mid-iteration. Caught it with
the same headless repro before it went anywhere near a PR, moved the
logic to itemChange(), and re-ran the full sequence -- select, resize,
undo, deselect, twice through -- clean.

Position: updateResizeHandlesPos() placed the handles on frameRect(),
which is a box sized to the text's natural (idealWidth()) content and
then re-centred inside boundingRect() -- it does not grow with
textWidth(). Once a text has been widened, frameRect() stays tight
around the glyphs while boundingRect() -- the box QGraphicsView actually
outlines as the selection, and the box a user drags relative to -- grows
around it, leaving the handles stranded well inside the visible
selection border. Fix: position them on boundingRect() instead, which
does track textWidth(); confirmed by widening a text and checking the
handle lands exactly on the new edge rather than partway across it.

Verified headlessly end to end on the original report's own element
("motor off" on grafcet.qet, folio 1): a single plain left-click (no
Shift, no right-click) now shows both handles at the true box border;
dragging resizes correctly and the handle tracks the growing edge;
Ctrl+Z restores the -1 auto-width sentinel and the handles stay at the
reverted position; clicking away removes them; repeated twice with no
crash. Qt 6.10.2, ctest 12/12, no new warnings in either changed file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

sources/qetgraphicsitem/dynamicelementtextitem.cpp

@@ -732,6 +732,16 @@ void DynamicElementTextItem::paint(QPainter *painter, const QStyleOptionGraphics
 {
 	DiagramTextItem::paint(painter, option, widget);
 
+		//Only ever repositions already-existing sibling items here --
+		//never adds or removes one. paint() runs while QGraphicsScene is
+		//iterating its item list to draw it, and mutating that list mid
+		//-iteration (which addResizeHandles()/removeResizeHandles() do,
+		//through QGraphicsScene::addItem()/removeItem()) crashes. An
+		//earlier version of this fix called them from here and crashed
+		//qelectrotech reproducibly on deselecting a text (SIGABRT); see
+		//refreshResizeHandlesVisibility() for where that now happens
+		//instead -- itemChange(), Qt's own safe hook for exactly this,
+		//already used below for this item's own selection.
 	if (m_left_resize_handle || m_right_resize_handle)
 		updateResizeHandlesPos();
 
@@ -826,10 +836,7 @@ QVariant DynamicElementTextItem::itemChange(QGraphicsItem::GraphicsItemChange ch
 	}
 	else if (change == QGraphicsItem::ItemSelectedHasChanged)
 	{
-		if (value.toBool())
-			addResizeHandles();
-		else
-			removeResizeHandles();
+		refreshResizeHandlesVisibility();
 	}
 	else if (change == QGraphicsItem::ItemSceneHasChanged && !scene())
 	{
@@ -878,6 +885,31 @@ bool DynamicElementTextItem::sceneEventFilter(QGraphicsItem *watched, QEvent *ev
 	return false;
 }
 
+/**
+	@brief DynamicElementTextItem::refreshResizeHandlesVisibility
+	Show the resize handles when this text is selected directly, OR when its
+	parent element is -- which is what an ordinary click without Shift
+	selects (DynamicElementTextItem::mousePressEvent() forwards a plain
+	click to the parent, so dragging a symbol by its label moves the whole
+	symbol; a pre-existing, unrelated behaviour, left untouched here).
+	Without this, the handles were reachable only via Shift+click or a
+	right-click's context menu, neither of which a user reaches for to
+	resize a text field (qelectrotech#591, reported by @arummler).
+
+	Called from itemChange() -- both this item's own ItemSelectedHasChanged,
+	below, and Element::itemChange() on the parent's, which calls this on
+	every one of its texts. Not from paint(): see the comment there for why
+	that crashed.
+*/
+void DynamicElementTextItem::refreshResizeHandlesVisibility()
+{
+	const bool handles_wanted = isSelected() || (m_parent_element && m_parent_element->isSelected());
+	if (handles_wanted && !m_left_resize_handle)
+		addResizeHandles();
+	else if (!handles_wanted && m_left_resize_handle)
+		removeResizeHandles();
+}
+
 /**
 	@brief DynamicElementTextItem::addResizeHandles
 	Create and show the two width-resize handles (left/right edge of
@@ -917,20 +949,32 @@ void DynamicElementTextItem::removeResizeHandles()
 
 /**
 	@brief DynamicElementTextItem::updateResizeHandlesPos
-	Keep the two resize handles at the vertical middle of frameRect()'s left
-	and right edges, in scene coordinates -- called on every paint() so it
-	stays correct across every kind of change that can move this item or
+	Keep the two resize handles at the vertical middle of boundingRect()'s
+	left and right edges, in scene coordinates -- called on every paint() so
+	it stays correct across every kind of change that can move this item or
 	change its size (position, rotation, font, text, textWidth...) without
 	needing a dedicated hook for each one.
+
+	Deliberately boundingRect(), not frameRect(): frameRect() is a tight box
+	around the text's own natural (idealWidth()) size, re-centred inside
+	boundingRect() -- it does not grow with textWidth(). Once a text has
+	been widened, that leaves a growing gap between the tight frame and the
+	dashed selection outline QGraphicsView draws at boundingRect(), which is
+	the box a user actually sees and expects a resize handle to sit on
+	(qelectrotech#591, reported by @arummler: "the drag elements should be
+	on the border of the box"). boundingRect() reflects the full
+	textWidth() (it is QGraphicsTextItem's own, driven by the document's
+	laid-out size), so the handles now track the box that is visibly
+	resized rather than the text glyphs inside it.
 */
 void DynamicElementTextItem::updateResizeHandlesPos()
 {
 	if (!m_left_resize_handle || !m_right_resize_handle)
 		return;
 
-	QRectF fr = frameRect();
-	m_left_resize_handle->setPos(mapToScene(QPointF(fr.left(), fr.center().y())));
-	m_right_resize_handle->setPos(mapToScene(QPointF(fr.right(), fr.center().y())));
+	QRectF br = boundingRect();
+	m_left_resize_handle->setPos(mapToScene(QPointF(br.left(), br.center().y())));
+	m_right_resize_handle->setPos(mapToScene(QPointF(br.right(), br.center().y())));
 }
 
 /**

sources/qetgraphicsitem/dynamicelementtextitem.h

@@ -121,6 +121,12 @@ class DynamicElementTextItem : public DiagramTextItem
 		void setRotationPointCenter(bool set);
 		bool rotationPointCenter() const;
 
+			//Called by Element::itemChange() when the PARENT's selection
+			//changes, so the parent can keep each of its texts' resize
+			//handles in sync with its own selection state. Public for that;
+			//see the .cpp for why it exists.
+		void refreshResizeHandlesVisibility();
+
 	protected:
 		void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
 		void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override;

sources/qetgraphicsitem/element.cpp

@@ -1664,6 +1664,31 @@ void Element::hoverLeaveEvent(QGraphicsSceneHoverEvent *e)
 	update();
 }
 
+/**
+	@brief Element::itemChange
+	On ItemSelectedHasChanged, tell each of this element's own dynamic texts
+	to re-check whether its resize handles should be showing --
+	DynamicElementTextItem::refreshResizeHandlesVisibility() shows them when
+	either the text itself or its parent (this) is selected. An ordinary
+	click with no Shift selects the parent, not the text
+	(DynamicElementTextItem::mousePressEvent() forwards it), so without this
+	a plain click on a symbol never showed the resize handles this PR adds
+	to its texts (qelectrotech#591, reported by @arummler) -- only
+	Shift+click or a right-click's context menu did, since those are the
+	paths that leave the text itself selected.
+*/
+QVariant Element::itemChange(GraphicsItemChange change, const QVariant &value)
+{
+	if (change == QGraphicsItem::ItemSelectedHasChanged)
+	{
+		const QList<DynamicElementTextItem *> texts = dynamicTextItems();
+		for (DynamicElementTextItem *deti : texts) {
+			deti->refreshResizeHandlesVisibility();
+		}
+	}
+	return QetGraphicsItem::itemChange(change, value);
+}
+
 /**
 	@brief Element::setUpFormula
 	Set up the formula used to create the label of this element

sources/qetgraphicsitem/element.h

@@ -255,6 +255,7 @@ class Element : public QetGraphicsItem
 				QGraphicsSceneMouseEvent *event) override;
 		void hoverEnterEvent(QGraphicsSceneHoverEvent *) override;
 		void hoverLeaveEvent(QGraphicsSceneHoverEvent *) override;
+		QVariant itemChange(GraphicsItemChange change, const QVariant &value) override;
 
 	protected:
 			//ATTRIBUTES related to linked element