From 414d7badf85d5449de232a1c9f488577eaa99926 Mon Sep 17 00:00:00 2001 From: "Jeremy [KK7GWY]" Date: Wed, 7 Oct 2026 15:33:47 -0700 Subject: [PATCH 1/2] fix(dvk): preserve quoted names ending in =. Principle VIII. --- src/core/backends/flex/CommandParser.cpp | 6 +++++- tests/dvk_model_status_test.cpp | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/core/backends/flex/CommandParser.cpp b/src/core/backends/flex/CommandParser.cpp index f6bf6d3abd..5b3045e007 100644 --- a/src/core/backends/flex/CommandParser.cpp +++ b/src/core/backends/flex/CommandParser.cpp @@ -21,7 +21,11 @@ QMap CommandParser::parseKVs(const QString& body) while (end < tokens.size() && !tokens[end].contains('"')) { ++end; } - if (end < tokens.size() && !tokens[end].contains(QLatin1String("=\""))) { + // A trailing quote closes this value even when its last character is '='. + // A quote followed by content instead begins the next quoted field. + if (end < tokens.size() + && (!tokens[end].contains(QLatin1String("=\"")) + || tokens[end].indexOf(QLatin1Char('"')) == tokens[end].size() - 1)) { for (qsizetype j = i + 1; j <= end; ++j) { token += QLatin1Char(' ') + tokens[j]; } diff --git a/tests/dvk_model_status_test.cpp b/tests/dvk_model_status_test.cpp index 736daf0b7b..52b0f38320 100644 --- a/tests/dvk_model_status_test.cpp +++ b/tests/dvk_model_status_test.cpp @@ -7,6 +7,7 @@ #include "models/RadioModel.h" #include +#include #include #include @@ -100,6 +101,22 @@ int main(int argc, char* argv[]) && kvs.value("name") == QStringLiteral("\"Solo\"") && kvs.contains("removed") && kvs.value("removed").isEmpty()); } + { + const QStringList names{QStringLiteral("CQ a="), QStringLiteral("CQ ="), + QStringLiteral("CQ a=b"), QStringLiteral("CQ a= b=")}; + for (const QString& name : names) { + const QString body = QStringLiteral("id=1 name=\"%1\" duration=1000").arg(name); + const QMap kvs = CommandParser::parseKVs(body); + report("equals signs inside a quoted name stay whole", + kvs.value("name") == QStringLiteral("\"%1\"").arg(name) + && kvs.value("duration") == QStringLiteral("1000") && kvs.size() == 3); + + DvkModel model; + feed(model, QStringLiteral("S629BA1D4|dvk added %1").arg(body)); + report("a DVK status preserves the full name and duration", + nameOf(model, 1) == name && durationOf(model, 1) == 1000); + } + } { // Review #6247: an unterminated value must not swallow a later quoted key. const auto kvs = CommandParser::parseKVs( From a00d6ec74383af517a0aff816559320ca44f15f5 Mon Sep 17 00:00:00 2001 From: "Jeremy [KK7GWY]" Date: Wed, 7 Oct 2026 15:47:03 -0700 Subject: [PATCH 2/2] fix(dvk): state the closing-quote rule exactly, and let Qt's own accessibility events stand alone. Principle VIII. Landing pass on #6264. Parser (maintainer: keep the token rule, tighten it) - The closing-quote test is a named predicate: a token closes the open value unless it opens another quoted field, and a token whose only quote is its last character always closes (endsWith && count == 1, which states what the first-quote-position test meant). - The function comment now describes that rule, including the one shape it cannot tell apart: on a malformed line a lone `label="` reads as the close, exactly as a valid name ending in '=' does. A new test pins that case, and that the keys after it survive. - The looped `=`-name assertions carry the name in their label, so a failure says which name broke. Accessibility (maintainer: fold the #6262 review finding into #6264) - setAccessibleName() and QLabel::setText() emit NameChanged themselves. The panel's extra coalesced F-key event and its manual status events are gone; F-key and row names are set only when they change. - The status label's accessible name carries the last announced state, so each state change is one NameChanged and the 10 Hz elapsed-time text and transfer progress add none. - dvk_panel_admission_test counts NameChanged through QAccessible::installUpdateHandler: a load of unchanged slots emits none on the F-keys, a rename emits one, a state change is one status event with no tick events after it, and disconnect emits one status event plus one for the renamed slot. Against the previous panel, three of those checks fail. dvk_model_status_test 56/56, dvk_panel_admission_test 11/11, dvk_panel_theme_test 33/33. Full ctest passes except control_receive_test, whose fix (#6263) landed on main after this branch's base. Architecture gates read the same on base and head. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/core/backends/flex/CommandParser.cpp | 19 ++++--- src/gui/DvkPanel.cpp | 51 ++++++------------- src/gui/DvkPanel.h | 5 -- tests/dvk_model_status_test.cpp | 19 +++++-- tests/dvk_panel_admission_test.cpp | 64 ++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 51 deletions(-) diff --git a/src/core/backends/flex/CommandParser.cpp b/src/core/backends/flex/CommandParser.cpp index 5b3045e007..e2159ab17a 100644 --- a/src/core/backends/flex/CommandParser.cpp +++ b/src/core/backends/flex/CommandParser.cpp @@ -10,9 +10,16 @@ QMap CommandParser::parseKVs(const QString& body) QMap result; // Body may look like: "freq=14.225000 mode=USB filter_lo=-1500 filter_hi=1500" // A quoted value may contain spaces (`dvk … name="CQ Contest"`) and is kept - // whole, quotes included, up to the token that closes it. A quote left - // open (no closing token before the next `key="` or the end of the line) - // splits as before, so a malformed value cannot swallow the keys after it. + // whole, quotes included, up to the token that closes it: the next token + // holding a quote, unless that token opens another quoted field + // (`key="text…`). A token whose only quote is its last character always + // closes, so a name may end in '=' (`name="CQ a="`); on a malformed line + // that same shape (`label="` alone) is read as the close. A quote left + // open splits as before, so it cannot swallow the keys after it. + const auto closesQuotedValue = [](const QString& token) { + return !token.contains(QLatin1String("=\"")) + || (token.endsWith(QLatin1Char('"')) && token.count(QLatin1Char('"')) == 1); + }; const QStringList tokens = body.split(' ', Qt::SkipEmptyParts); for (qsizetype i = 0; i < tokens.size(); ++i) { QString token = tokens[i]; @@ -21,11 +28,7 @@ QMap CommandParser::parseKVs(const QString& body) while (end < tokens.size() && !tokens[end].contains('"')) { ++end; } - // A trailing quote closes this value even when its last character is '='. - // A quote followed by content instead begins the next quoted field. - if (end < tokens.size() - && (!tokens[end].contains(QLatin1String("=\"")) - || tokens[end].indexOf(QLatin1Char('"')) == tokens[end].size() - 1)) { + if (end < tokens.size() && closesQuotedValue(tokens[end])) { for (qsizetype j = i + 1; j <= end; ++j) { token += QLatin1Char(' ') + tokens[j]; } diff --git a/src/gui/DvkPanel.cpp b/src/gui/DvkPanel.cpp index fcb29f4dbe..97438d7b52 100644 --- a/src/gui/DvkPanel.cpp +++ b/src/gui/DvkPanel.cpp @@ -119,6 +119,10 @@ DvkPanel::DvkPanel(DvkModel* model, QWidget* parent) statusRow->addWidget(m_statusDot, 0, Qt::AlignTop); m_statusLabel = new QLabel("Idle"); m_statusLabel->setObjectName(QStringLiteral("dvkStatus")); + // A QLabel with no accessible name announces every setText(). This one's + // name carries the last announced state, so the 10 Hz elapsed-time text is + // silent and each state change is one NameChanged (announceStatus). + m_statusLabel->setAccessibleName(QStringLiteral("Idle")); m_statusLabel->setWordWrap(true); m_statusLabel->setAlignment(Qt::AlignLeft | Qt::AlignTop); // Two lines are reserved so a wrapped refusal never shifts the slots. @@ -359,11 +363,6 @@ DvkPanel::DvkPanel(DvkModel* model, QWidget* parent) stopActiveOperation(); }); - m_announceTimer = new QTimer(this); - m_announceTimer->setSingleShot(true); - m_announceTimer->setInterval(150); - connect(m_announceTimer, &QTimer::timeout, this, &DvkPanel::flushSlotAnnouncement); - // Elapsed timer for recording/playback/preview progress m_elapsedTimer = new QTimer(this); m_elapsedTimer->setInterval(100); @@ -397,36 +396,20 @@ void DvkPanel::updateSlotAccessibility(int id, const QString& name, int duration { const int idx = id - 1; const QString length = durationMs > 0 ? formatDuration(durationMs) : QStringLiteral("empty"); - m_rowFrames[idx]->setAccessibleName(QString("Slot %1: %2, %3").arg(id).arg(name, length)); + // setAccessibleName() emits NameChanged itself, so names are set only when + // they change: a rename is one event, and a reload of unchanged slots none. + const auto setName = [](QWidget* widget, const QString& name) { + if (widget->accessibleName() != name) { + widget->setAccessibleName(name); + } + }; + setName(m_rowFrames[idx], QString("Slot %1: %2, %3").arg(id).arg(name, length)); auto* play = m_fkeyBtns[idx]; - const QString playName = QString("Play slot %1: %2").arg(id).arg(name); - const bool changed = play->accessibleName() != playName; - play->setAccessibleName(playName); + setName(play, QString("Play slot %1: %2").arg(id).arg(name)); play->setAccessibleDescription(durationMs > 0 ? QString("%1 recording. Transmits on the transmit slice (F%2).").arg(length).arg(id) : QStringLiteral("Empty slot, nothing to play.")); - // Announce a real change the operator can see, one slot at a time. A - // load or reset changes many slots at once; flushSlotAnnouncement() stays - // silent for those instead of reading a dozen names back to back. - if (changed && m_announceTimer) { - if (!m_announceSlots.contains(id)) { - m_announceSlots.append(id); - } - m_announceTimer->start(); - } -} - -void DvkPanel::flushSlotAnnouncement() -{ - if (m_announceSlots.size() == 1) { - QPushButton* play = m_fkeyBtns[m_announceSlots.first() - 1]; - if (play->isVisible()) { - QAccessibleEvent ev(play, QAccessible::NameChanged); - QAccessible::updateAccessibility(&ev); - } - } - m_announceSlots.clear(); } void DvkPanel::announceStatus(const QString& text, bool error) @@ -436,8 +419,7 @@ void DvkPanel::announceStatus(const QString& text, bool error) return; } m_statusLabel->setText(text); - QAccessibleEvent ev(m_statusLabel, QAccessible::NameChanged); - QAccessible::updateAccessibility(&ev); + m_statusLabel->setAccessibleName(text); // emits the one NameChanged } void DvkPanel::togglePlayback(int id) @@ -563,11 +545,10 @@ void DvkPanel::onStatusChanged(int status, int id) m_elapsedTimer->start(); // Announce the new operation once; the 100 ms tick that follows - // only rewrites the elapsed time and stays silent. + // only rewrites the visible elapsed time. setStyleProperty(m_statusLabel, "tone", QString()); onElapsedTick(); - QAccessibleEvent ev(m_statusLabel, QAccessible::NameChanged); - QAccessible::updateAccessibility(&ev); + m_statusLabel->setAccessibleName(m_statusLabel->text()); } else { onElapsedTick(); } diff --git a/src/gui/DvkPanel.h b/src/gui/DvkPanel.h index 6e2e1e3a43..56c3b8b4f5 100644 --- a/src/gui/DvkPanel.h +++ b/src/gui/DvkPanel.h @@ -70,11 +70,6 @@ private slots: // (#2464, #2582). QVector m_shortcuts; - // F-key name changes collected for one announcement; a burst (slot load, - // disconnect) of more than one is not announced. - QVector m_announceSlots; - QTimer* m_announceTimer{nullptr}; - void flushSlotAnnouncement(); void selectSlot(int id); // Row and F-key names carry the slot's name and length for screen readers. diff --git a/tests/dvk_model_status_test.cpp b/tests/dvk_model_status_test.cpp index 52b0f38320..28b9e212b3 100644 --- a/tests/dvk_model_status_test.cpp +++ b/tests/dvk_model_status_test.cpp @@ -107,16 +107,29 @@ int main(int argc, char* argv[]) for (const QString& name : names) { const QString body = QStringLiteral("id=1 name=\"%1\" duration=1000").arg(name); const QMap kvs = CommandParser::parseKVs(body); - report("equals signs inside a quoted name stay whole", + const QByteArray parsed = + QStringLiteral("equals signs inside the quoted name '%1' stay whole").arg(name).toUtf8(); + report(parsed.constData(), kvs.value("name") == QStringLiteral("\"%1\"").arg(name) && kvs.value("duration") == QStringLiteral("1000") && kvs.size() == 3); DvkModel model; feed(model, QStringLiteral("S629BA1D4|dvk added %1").arg(body)); - report("a DVK status preserves the full name and duration", - nameOf(model, 1) == name && durationOf(model, 1) == 1000); + const QByteArray modelled = + QStringLiteral("a DVK status keeps the name '%1' and its duration").arg(name).toUtf8(); + report(modelled.constData(), nameOf(model, 1) == name && durationOf(model, 1) == 1000); } } + { + // The one shape the closing rule cannot tell apart: on a malformed line, + // a `key="` token with nothing after its quote reads as the close (it is + // exactly how a valid name ending in '=' looks). Keys after it survive. + const auto kvs = CommandParser::parseKVs( + QStringLiteral("name=\"abc label=\" x y\" baz=2")); + report("a lone trailing-quote token closes an open value; later keys survive", + kvs.value("name") == QStringLiteral("\"abc label=\"") + && kvs.value("baz") == QStringLiteral("2")); + } { // Review #6247: an unterminated value must not swallow a later quoted key. const auto kvs = CommandParser::parseKVs( diff --git a/tests/dvk_panel_admission_test.cpp b/tests/dvk_panel_admission_test.cpp index 2492007ae5..8d0a6998ad 100644 --- a/tests/dvk_panel_admission_test.cpp +++ b/tests/dvk_panel_admission_test.cpp @@ -7,7 +7,11 @@ #include "gui/DvkPanel.h" #include "models/DvkModel.h" +#include #include +#include +#include +#include #include #include @@ -35,6 +39,24 @@ void feed(DvkModel& model, const QString& line) model.applyStatus(msg.object, msg.kvs); } +// NameChanged events per widget, through Qt's accessibility update hook. +QHash g_nameChanges; +void countNameChanges(QAccessibleEvent* event) +{ + if (event->type() == QAccessible::NameChanged) { + ++g_nameChanges[event->object()]; + } +} + +void spin(int ms) +{ + QElapsedTimer t; + t.start(); + while (t.elapsed() < ms) { + QApplication::processEvents(QEventLoop::AllEvents, 20); + } +} + void seed(DvkModel& model) { feed(model, QStringLiteral("S1|dvk status=idle enabled=1")); @@ -101,6 +123,48 @@ int main(int argc, char** argv) sent == QStringList{QStringLiteral("dvk rec_start id=1")}); } + { + // Review #6262: setAccessibleName() and QLabel::setText() emit their own + // NameChanged. The panel adds none of its own, so each real change is + // one event, unchanged reloads are none, and the elapsed tick is silent. + QAccessible::setActive(true); + QAccessible::installUpdateHandler(countNameChanges); + DvkModel model; + DvkPanel panel(&model); + panel.show(); + auto* f3 = panel.findChild(QStringLiteral("dvkPlaySlot3")); + auto* status = panel.findChild(QStringLiteral("dvkStatus")); + + g_nameChanges.clear(); + seed(model); + int fkeyEvents = 0; + for (int id = 1; id <= 12; ++id) { + fkeyEvents += g_nameChanges.value( + panel.findChild(QStringLiteral("dvkPlaySlot%1").arg(id))); + } + report("loading slots whose names did not change emits no F-key NameChanged", + fkeyEvents == 0); + + g_nameChanges.clear(); + feed(model, QStringLiteral("S1|dvk id=3 name=\"CQ Contest\" duration=4210")); + spin(300); // past any deferred announcement + report("a rename emits exactly one NameChanged on its F-key", + g_nameChanges.value(f3) == 1); + + g_nameChanges.clear(); + feed(model, QStringLiteral("S1|dvk status=preview id=3 enabled=1")); + spin(550); // five elapsed-time ticks + report("a state change is one status NameChanged; the elapsed tick adds none", + g_nameChanges.value(status) == 1); + + g_nameChanges.clear(); + feed(model, QStringLiteral("S0|dvk status=idle enabled=1")); + model.reset(); + report("disconnect announces the state once and only the renamed slot's F-key", + g_nameChanges.value(status) == 1 && g_nameChanges.value(f3) == 1); + QAccessible::installUpdateHandler(nullptr); + } + { // The bridge refuses controls marked as keying TX. DvkModel model;