m_first_move was initialized before _linestyle in the constructor
initializer list, but _linestyle is declared first in the class. Reorder
to match declaration order.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When an item type is selected for the first time the properties dock
expands, causing the QGraphicsView viewport to shrink. Qt recalculates
scene coordinates and fires one or more synthetic mouseMoveEvents before
the user has actually moved the mouse.
The original code used a single-shot m_first_move flag in
CustomElementGraphicPart, which absorbed exactly one spurious event.
PartText and PartDynamicTextField had no protection at all.
Fix: compare screen-coordinate displacement against
QApplication::startDragDistance() (~4 px). Screen coordinates are
stable across viewport resizes, so the check correctly rejects
synthetic dock-expansion events while allowing genuine drags.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same root cause as ProjectPropertiesDialog: Qt::WindowModal only blocks the
direct parent window, leaving the rest of the MDI area live. If new_project
or close_project fires while the app settings dialog is open, any raw pointers
derived from the project list become stale. Switch to ApplicationModal to
block all windows for the duration of the dialog.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ProjectPropertiesDialog::exec() was using Qt::WindowModal, which only
blocks the parent ProjectView window. The MDI workspace and its other
subwindows remained interactive, so actions like new_project or
close_project could fire while the dialog's config pages still held raw
QETProject* pointers — leading to a SIGSEGV when Qt's event loop later
dispatched a signal through one of those stale pointers.
Detected by the 8-hour GUI fuzzer: action sequence add_diagram_page →
flood_wires ×18 → new_project while Project Properties was open
produced exit code -11 (SIGSEGV) on the first of 12,717 actions.
Switch to Qt::ApplicationModal so no window can receive input while the
dialog is open. Project Properties is a short-lived dialog; blocking
the whole application for its duration matches user expectation and
removes the lifetime hazard without requiring QPointer surgery across
four config-page classes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Follow-up to the eventFilter fix: testing showed double-click works
when QET is already running, but a cold launch (app not running yet)
still opens an empty window. Finder/Launch Services can deliver the
QFileOpenEvent to the QApplication's native event loop before main()
reaches the point where QETApp is constructed and its real eventFilter
is installed -- there's a window between 'SingleApplication app(...)'
and 'QETApp qetapp;' during which the event can arrive and be lost.
Add a minimal EarlyFileOpenCatcher installed on app immediately after
it's constructed (before anything else can run an event loop). It only
buffers the file path. Once QETApp exists, main() swaps it out for the
real QETApp::eventFilter and drains anything that was buffered via
qetapp.openFiles(), so no cold-launch QFileOpenEvent is silently
dropped.
Confirmed by manual testing:
- 'open app --args file' on .qet/.elmt/.titleblock: OK (already worked)
- double-click while app already running: OK (already worked)
- double-click cold launch: previously opened an empty window, this
buffers and replays the event so it now opens the right editor.
Root cause (see issue #218 discussion):
- QETApp::eventFiltrer (Q_OS_DARWIN) was correct in intent but never
actually installed as an event filter anywhere, and its name didn't
match the QObject::eventFilter virtual signature, so even if it had
been installed it would never have been invoked as an override.
Confirmed dead code.
- main.cpp instead routed Finder's QFileOpenEvent through
MacOSXOpenEvent -> SingleApplication::sendMessage(), which is the
secondary-to-primary IPC channel. Called from within the primary
instance itself, sendMessage() fails silently and the file path is
dropped, which is exactly what double-click does (Finder doesn't spawn
a secondary process, the running instance receives the FileOpen event
directly).
- openFiles() takes a QETArguments, not a QStringList. The dead code's
openFiles(QStringList() << filename) only worked via an untested
implicit QStringList -> QList<QString> -> QETArguments conversion
chain.
Fix:
- Rename eventFiltrer -> eventFilter (qetapp.h/.cpp) so it's a proper
override of QObject::eventFilter, and make it public so main.cpp can
install it on QApplication.
- Build the QETArguments explicitly instead of relying on implicit
conversion.
- main.cpp: drop MacOSXOpenEvent entirely (include + instantiation),
install qetapp as the Q_OS_MACOS event filter on app right after
QETApp qetapp; is constructed and before app.exec(). No race window:
the event loop hasn't started, so no QFileOpenEvent can be delivered
before the filter is installed.
QETArguments::handleFileArgument() already sorts files by extension
(.elmt, titleblock, else project) and openFiles() already fans out to
openProjectFiles()/openElementFiles()/openTitleBlockTemplateFiles()
accordingly, so this single fix covers .qet, .elmt and .titleblock
double-click/drag-to-dock on macOS, not just .qet.
SingleApplication's sendMessage/receivedMessage flow (used for CLI args
on all platforms) is untouched; this only touches the Q_OS_MACOS block,
so there is no behavior change on Windows/Linux.
Follow-up (not included here): the macOS Info.plist (misc/Info.plist)
has an empty CFBundleShortVersionString, which may affect macOS's
willingness to retain file associations across app updates.
The default user-collection path ends in "elements" and the default
company-collection path ends in "elements-company". Both
FileElementCollectionItem::isCustomCollection() and
ElementsLocation::isCustomCollection() used startsWith(customDir),
so "…/elements-company/…" matched "…/elements" and returned true.
This caused ElementsCollectionModel::addLocation() to insert a
newly-saved user-collection element as a child of the company-
collection branch in the tree, making it appear in the wrong panel.
Fix: require the path to equal the directory root exactly, or to
start with the directory root followed by '/'.
path == dir || path.startsWith(dir + QLatin1Char('/'))
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Terminal stores its TerminalData* as member d but never deletes it.
Every Element creation (placing on diagram, loading icon for the
element browser, drag previews) leaks one TerminalData per terminal.
ASan confirmed 112 leaked objects (9856 bytes) in a short session
across four call sites all rooted in Element::parseTerminal.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Qt 5.15.x added libxcb-cursor0 as a hard runtime dependency of the xcb
platform plugin (libqxcb.so). The kf5-5-110-qt-5-15-11-core22 content
snap does not bundle this library, so when the snap runs on an Ubuntu 24.04
host the dlopen() of the plugin fails with:
qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in ""
even though it was found.
Staging libxcb-cursor0 from the Ubuntu 22.04 archive satisfies the
dependency without changing the snap base, Qt version, or any other
dependency. No ABI mismatch: the plugin and the staged library are
both built against the core22 (22.04) ABI.
Fixes issue #373.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1. machine_info.h: zero-initialise Screen struct members
Max_width, Max_height, count, width[] and height[] were bare
int32_t with no initialiser. The comparisons in
init_get_Screen_info() read them before any write, producing
undefined behaviour flagged by Valgrind as 'Conditional jump
or move depends on uninitialised value(s)'.
2. main.cpp: pre-initialise MachineInfo on the main thread
MachineInfo::instance() was first called inside QtConcurrent::run(),
causing its constructor (which calls qApp->screens()) to run on a
background thread. QScreen methods are not thread-safe in Qt5.
Calling instance() once on the main thread before the worker
launches guarantees the singleton is fully built first; subsequent
calls from the worker just return the cached pointer.
3. qetdiagrameditor.h: move m_first_show before the QActionGroup members
C++ initialises members in declaration order. m_first_show was
declared after the QActionGroup members (line 256 vs 168). During
construction of m_row_column_actions_group(this), Qt dispatches a
QObject parent-change event that reaches QETDiagramEditor::event(),
which reads m_first_show before it has been initialised.
Moving the declaration to the top of the first private: block
ensures it is initialised before any member that can trigger events.
All three found via Valgrind --tool=memcheck on Ubuntu 22.04 / Qt 5.15.3.
Relates-to: PR #514 (same QtConcurrent thread-safety pattern).
Reviewer requested configdir/datadir instead of cached for consistency
with the surrounding code style.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
QStandardPaths::writableLocation() is not thread-safe in Qt5.
ElementsCollectionModel::reload() launches:
QtConcurrent::map(m_items_list_to_setUp, setUpData)
Each worker calls FileElementCollectionItem::setUpData()
→ collectionPath() → isCollectionRoot()
→ QETApp::userMacrosDir() → QETApp::dataDir()
→ QStandardPaths::writableLocation() ← SIGSEGV (null deref)
The crash was confirmed by Valgrind (address 0x0, inside
libQt5Core's writableLocation internals).
Fix: replace the bare QStandardPaths calls in dataDir() and
configDir() with a C++11 static-local lambda. The compiler
guarantees the lambda body runs exactly once across all threads
(magic statics, ISO C++11 §6.7). After the first (main-thread)
call the result is returned lock-free.
Relates-to: #492 (same QtConcurrent lifetime pattern fixed in
QETProject::writeBackup by PR #512).
Although I haven’t encountered the problems described myself (nor do I need to!), the changes and additions look plausible and compile without errors or warnings, so I’m approving this PR!
But as a remark:
I'm not particularly familiar with the Qt functions used here. But when I see that there is a version-specific implementation for Qt5 and only a debug-message mentioned for Qt6, it makes me wonder:
Is it implemented differently there, or is it not even needed there — which I don't think is the case...
If possible, "we" should also include something for Qt6 and later versions in another PR.
Do you know what’s needed for Qt6, @ispyisail ?
writeBackup() fires QtConcurrent::run(QET::writeToFile, ..., &m_backup_file)
fire-and-forget: the QFuture was discarded and nothing kept m_backup_file
alive until the worker finished. If the QETProject was destroyed first, the
worker wrote through the freed member -> use-after-free crash in
QET::writeToFile (intermittent; ~1/6 on short-lived CLI runs).
Store the QFuture and waitForFinished() in ~QETProject (and before
setFilePath() re-points the managed backup file). Also skip launching a new
backup while one is still running, so two threads never write m_backup_file
at once.
The Qt6 path is still a TODO stub and the QtConcurrent block is KF5-only, so
this affects only the Qt5/KF5 build that actually has the backup code.
Per review (plc-user): scope the reset to items currently painted with the
red Dense4Pattern instead of clearing every item's background. This avoids
clobbering other backgrounds (e.g. the amber "show this dir" highlight)
and skips needless item updates on large collections.
ElementsCollectionModel::highlightUnusedElement() only ever painted the
currently-unused elements red; it never cleared the background of items
that were no longer unused. So when an element was re-added to a project
and saved, its red 'unused' highlight persisted until the model was
rebuilt from scratch.
Reset every item's background before re-applying the highlight to the
current unused set.
The Windows search list for the qet_tb_generator plugin included
`~/Application Data/qet/qet_tb_generator.exe` as a fallback. That legacy
junction path is the same inaccessible location the standard-directories
change moved QET away from, and it never matched a pip install anyway:
`pip install qet_tb_generator` puts the executable in the Python Scripts
directory (`...\PythonXX\Scripts` on PATH, or
`%APPDATA%\Python\PythonXX\Scripts` for --user installs), not in
`QETApp::dataDir()`.
Pip installs are already found via QStandardPaths::findExecutable (PATH),
and manual binary drops via dataDir()/binary and the working directory.
The legacy entry only matched old manual drops into the inaccessible
folder, so remove it.
Refs qelectrotech-source-mirror#199
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Without a source-tag or source-commit, snapcraft pulls the latest
HEAD of qet_tb_generator-plugin on every build. This makes builds
non-reproducible and risks breakage whenever the upstream repo changes.
Pin to the only published release tag (v1.31, commit d6ee3cf) so
the snap always builds against a known-good version of the plugin.
Closes#202
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The .desktop, MIME package, and appdata install rules are
freedesktop.org conventions and only apply on Linux. Wrapping
them in if(UNIX AND NOT APPLE) prevents a configure failure on
macOS and Windows where QET_APPDATA_PATH and QET_MIME_PACKAGE_PATH
are not defined.
Also replace the hardcoded share/mime/packages path with
${QET_MIME_PACKAGE_PATH} for consistency with
paths_compilation_installation.cmake.
No change to Linux build behaviour.
Laurent Trinques