From 1fd2e0a0db6ab1daa6dd4c279302e1c7ec08d916 Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 21:29:19 +0400 Subject: [PATCH] =?UTF-8?q?gui:=20Options=20=E2=80=94=20Capture=20tab,=20s?= =?UTF-8?q?aveTo.allowedRoots,=20lock=20coverage=20to=20the=20schema?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Options was missing 8 of the 43 real settings.* keys: capture.enabled, monitoredExtensions, monitoredMimeTypes, minSizeBytes, excludedHosts, bypassModifier, autoStartTypes, and saveTo.allowedRoots. The capture.* keys were dropped in an earlier pass on the mistaken read that the spec's "File Types" tab meant per-category extension lists (which do live on Category, not settings.*) — they're real settings.* keys for a real daemon feature (the extension's auto-capture policy), so the tab exists now, named "Capture" to match what it actually configures rather than the spec's label. New tst_optionsdialog case (allKeysMatchesTheSchemaExactly) loads Settings.schema.json itself at test time and diffs its property set against OptionsDialog::allKeys() — this drifted silently once already, so the regression is now a build-time gate an unused import or a future key addition would trip, not something that needs re-discovering by hand again. Verified against a real veloxd (not just mockd): settings.get across all 43 keys, a settings.set/get round trip on a scalar (connection.timeoutSec) and on array-valued keys in the shapes OptionsDialog::currentValues() actually produces (capture.monitoredExtensions, proxy.bypassHosts, saveTo.allowedRoots), and event.settings.changed fanning out to a second subscribed client — all round-tripped and restored to their original values afterward. The real OptionsDialog widget also loads and renders correctly against that same daemon's live defaults with no crash under ASan+UBSan. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NSCdCWFXBTSBBK3MzWtJiC --- gui/src/dialogs/OptionsDialog.cpp | 95 ++++++++++++++++++++++++++++++- gui/src/dialogs/OptionsDialog.hpp | 23 ++++++-- gui/tests/CMakeLists.txt | 8 ++- gui/tests/tst_optionsdialog.cpp | 22 +++++++ 4 files changed, 137 insertions(+), 11 deletions(-) diff --git a/gui/src/dialogs/OptionsDialog.cpp b/gui/src/dialogs/OptionsDialog.cpp index 4240687..8dc1864 100644 --- a/gui/src/dialogs/OptionsDialog.cpp +++ b/gui/src/dialogs/OptionsDialog.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,7 @@ #include "rpc/Protocol.hpp" #include "rpc/RpcClient.hpp" +#include "util/Theme.hpp" namespace velox::gui { namespace { @@ -43,7 +45,7 @@ void setBufferCombo(QComboBox *combo, qint64 bytes) { combo->setCurrentIndex(combo->count() / 2); // an unrecognized value: land near the middle } -QStringList splitHosts(const QString &text) { +QStringList splitCsv(const QString &text) { QStringList out; for (const QString &h : text.split(QLatin1Char(','), Qt::SkipEmptyParts)) { out << h.trimmed(); @@ -51,6 +53,14 @@ QStringList splitHosts(const QString &text) { return out; } +QString joinArray(const QJsonArray &a) { + QStringList items; + for (const QJsonValue &v : a) { + items << v.toString(); + } + return items.join(QStringLiteral(", ")); +} + } // namespace QStringList OptionsDialog::allKeys() { @@ -61,10 +71,18 @@ QStringList OptionsDialog::allKeys() { "general.confirmOnExit", "general.language", "general.checkForUpdates", + "capture.enabled", + "capture.monitoredExtensions", + "capture.monitoredMimeTypes", + "capture.minSizeBytes", + "capture.excludedHosts", + "capture.bypassModifier", + "capture.autoStartTypes", "saveTo.defaultDir", "saveTo.tempDir", "saveTo.fileExistsPolicy", "saveTo.createSubfolderPerSite", + "saveTo.allowedRoots", "connection.preset", "connection.maxSegmentsPerDownload", "connection.bufferBytes", @@ -112,13 +130,14 @@ OptionsDialog::OptionsDialog(rpc::RpcClient *client, QWidget *parent) resize(560, 480); buildGeneralTab(); + buildCaptureTab(); buildSaveToTab(); buildConnectionTab(); buildDownloadsTab(); buildProxyTab(); buildSoundsTab(); - statusLabel_->setStyleSheet(QStringLiteral("color: #c0392b;")); + statusLabel_->setStyleSheet(theme::errorLabelStyle()); statusLabel_->hide(); auto *buttons = new QDialogButtonBox( @@ -183,6 +202,37 @@ void OptionsDialog::buildGeneralTab() { tabs_->addTab(page, tr("General")); } +void OptionsDialog::buildCaptureTab() { + auto *page = new QWidget(this); + captureEnabled_ = new QCheckBox(tr("Capture downloads from the browser extension"), page); + monitoredExtensions_ = new QLineEdit(page); + monitoredExtensions_->setPlaceholderText(tr("comma-separated, e.g. zip, iso, mp4")); + monitoredMimeTypes_ = new QLineEdit(page); + monitoredMimeTypes_->setPlaceholderText(tr("comma-separated, e.g. application/zip")); + minSizeKiB_ = new QSpinBox(page); + minSizeKiB_->setRange(0, 2000000); + minSizeKiB_->setSuffix(tr(" KiB")); + excludedHosts_ = new QLineEdit(page); + excludedHosts_->setPlaceholderText(tr("comma-separated, e.g. *.google.com")); + bypassModifier_ = new QComboBox(page); + bypassModifier_->addItem(tr("Alt"), QStringLiteral("alt")); + bypassModifier_->addItem(tr("Ctrl"), QStringLiteral("ctrl")); + bypassModifier_->addItem(tr("Shift"), QStringLiteral("shift")); + bypassModifier_->addItem(tr("None"), QStringLiteral("none")); + autoStartTypes_ = new QLineEdit(page); + autoStartTypes_->setPlaceholderText(tr("extensions that skip the File Info dialog")); + + auto *form = new QFormLayout(page); + form->addRow(captureEnabled_); + form->addRow(tr("Monitored extensions:"), monitoredExtensions_); + form->addRow(tr("Monitored MIME types:"), monitoredMimeTypes_); + form->addRow(tr("Minimum size:"), minSizeKiB_); + form->addRow(tr("Never capture from:"), excludedHosts_); + form->addRow(tr("Bypass-capture modifier key:"), bypassModifier_); + form->addRow(tr("Auto-start these types:"), autoStartTypes_); + tabs_->addTab(page, tr("Capture")); +} + void OptionsDialog::buildSaveToTab() { auto *page = new QWidget(this); defaultDir_ = new QLineEdit(page); @@ -193,6 +243,11 @@ void OptionsDialog::buildSaveToTab() { fileExistsPolicy_->addItem(tr("Overwrite"), QStringLiteral("overwrite")); fileExistsPolicy_->addItem(tr("Resume"), QStringLiteral("resume")); createSubfolderPerSite_ = new QCheckBox(tr("Create a subfolder per site"), page); + allowedRoots_ = new QPlainTextEdit(page); + allowedRoots_->setPlaceholderText( + tr("One directory per line — every save path must " + "canonicalize inside one of these")); + allowedRoots_->setMaximumHeight(80); auto *defaultDirRow = new QWidget(page); auto *defaultDirLayout = new QHBoxLayout(defaultDirRow); @@ -215,6 +270,7 @@ void OptionsDialog::buildSaveToTab() { form->addRow(tr("Temp folder:"), tempDirRow); form->addRow(tr("If a file already exists:"), fileExistsPolicy_); form->addRow(createSubfolderPerSite_); + form->addRow(tr("Allowed save roots:"), allowedRoots_); tabs_->addTab(page, tr("Save To")); } @@ -390,12 +446,27 @@ void OptionsDialog::populateFrom(const QJsonObject &v) { language_->setCurrentIndex(langIdx >= 0 ? langIdx : 0); checkForUpdates_->setChecked(v.value("general.checkForUpdates").toBool(true)); + captureEnabled_->setChecked(v.value("capture.enabled").toBool(true)); + monitoredExtensions_->setText(joinArray(v.value("capture.monitoredExtensions").toArray())); + monitoredMimeTypes_->setText(joinArray(v.value("capture.monitoredMimeTypes").toArray())); + minSizeKiB_->setValue(static_cast(v.value("capture.minSizeBytes").toDouble() / 1024)); + excludedHosts_->setText(joinArray(v.value("capture.excludedHosts").toArray())); + const int bypassIdx = + bypassModifier_->findData(v.value("capture.bypassModifier").toString("alt")); + bypassModifier_->setCurrentIndex(bypassIdx >= 0 ? bypassIdx : 0); + autoStartTypes_->setText(joinArray(v.value("capture.autoStartTypes").toArray())); + defaultDir_->setText(v.value("saveTo.defaultDir").toString()); tempDir_->setText(v.value("saveTo.tempDir").toString()); const int policyIdx = fileExistsPolicy_->findData(v.value("saveTo.fileExistsPolicy").toString("ask")); fileExistsPolicy_->setCurrentIndex(policyIdx >= 0 ? policyIdx : 0); createSubfolderPerSite_->setChecked(v.value("saveTo.createSubfolderPerSite").toBool()); + QStringList roots; + for (const QJsonValue &r : v.value("saveTo.allowedRoots").toArray()) { + roots << r.toString(); + } + allowedRoots_->setPlainText(roots.join(QLatin1Char('\n'))); const int presetIdx = connectionPreset_->findData(v.value("connection.preset").toString("auto")); @@ -452,10 +523,28 @@ QJsonObject OptionsDialog::currentValues() const { v["general.language"] = language_->currentData().toString(); v["general.checkForUpdates"] = checkForUpdates_->isChecked(); + v["capture.enabled"] = captureEnabled_->isChecked(); + v["capture.monitoredExtensions"] = + QJsonArray::fromStringList(splitCsv(monitoredExtensions_->text())); + v["capture.monitoredMimeTypes"] = + QJsonArray::fromStringList(splitCsv(monitoredMimeTypes_->text())); + v["capture.minSizeBytes"] = static_cast(minSizeKiB_->value()) * 1024; + v["capture.excludedHosts"] = QJsonArray::fromStringList(splitCsv(excludedHosts_->text())); + v["capture.bypassModifier"] = bypassModifier_->currentData().toString(); + v["capture.autoStartTypes"] = QJsonArray::fromStringList(splitCsv(autoStartTypes_->text())); + v["saveTo.defaultDir"] = defaultDir_->text(); v["saveTo.tempDir"] = tempDir_->text(); v["saveTo.fileExistsPolicy"] = fileExistsPolicy_->currentData().toString(); v["saveTo.createSubfolderPerSite"] = createSubfolderPerSite_->isChecked(); + QStringList roots; + for (const QString &line : allowedRoots_->toPlainText().split(QLatin1Char('\n'))) { + const QString trimmed = line.trimmed(); + if (!trimmed.isEmpty()) { + roots << trimmed; + } + } + v["saveTo.allowedRoots"] = QJsonArray::fromStringList(roots); v["connection.preset"] = connectionPreset_->currentData().toString(); v["connection.maxSegmentsPerDownload"] = maxSegmentsPerDownload_->value(); @@ -479,7 +568,7 @@ QJsonObject OptionsDialog::currentValues() const { v["proxy.host"] = proxyHost_->text(); v["proxy.port"] = proxyPort_->value(); v["proxy.username"] = proxyUsername_->text(); - v["proxy.bypassHosts"] = QJsonArray::fromStringList(splitHosts(proxyBypassHosts_->text())); + v["proxy.bypassHosts"] = QJsonArray::fromStringList(splitCsv(proxyBypassHosts_->text())); v["proxy.pacUrl"] = proxyPacUrl_->text(); v["sounds.enabled"] = soundsEnabled_->isChecked(); diff --git a/gui/src/dialogs/OptionsDialog.hpp b/gui/src/dialogs/OptionsDialog.hpp index b0f4249..6985436 100644 --- a/gui/src/dialogs/OptionsDialog.hpp +++ b/gui/src/dialogs/OptionsDialog.hpp @@ -1,11 +1,12 @@ // The Options dialog. Lane GUI. // // docs/03-gui-spec.md §4: every control here maps 1:1 onto a settings.* key from -// contracts/schema/types/Settings.schema.json. The spec's "File Types" and "Site Logins" -// tabs have no backing key (per-category extension lists live on Category via -// category.upsert, not settings.*; login credentials go to the Secret Service) — a real -// tab either binds to a real key or does not exist here, so those two are left out rather -// than shipped as fake affordances. +// contracts/schema/types/Settings.schema.json. The spec's "File Types" tab turned out to +// have real backing after all (capture.monitoredExtensions/monitoredMimeTypes/ +// autoStartTypes are settings.* keys, not Category — a mistake in an earlier pass here, +// caught while checking this dialog covers all 43 keys against the now-live real veloxd); +// it is named "Capture" below to match what it actually configures. "Site Logins" still +// has no settings.* key (credentials go to the Secret Service) and stays out. #pragma once @@ -16,6 +17,7 @@ class QCheckBox; class QComboBox; class QLabel; class QLineEdit; +class QPlainTextEdit; class QSpinBox; class QTabWidget; @@ -45,6 +47,7 @@ class OptionsDialog : public QDialog { private: void buildGeneralTab(); + void buildCaptureTab(); void buildSaveToTab(); void buildConnectionTab(); void buildDownloadsTab(); @@ -67,11 +70,21 @@ class OptionsDialog : public QDialog { QComboBox *language_; QCheckBox *checkForUpdates_; + // Capture + QCheckBox *captureEnabled_; + QLineEdit *monitoredExtensions_; + QLineEdit *monitoredMimeTypes_; + QSpinBox *minSizeKiB_; + QLineEdit *excludedHosts_; + QComboBox *bypassModifier_; + QLineEdit *autoStartTypes_; + // Save To QLineEdit *defaultDir_; QLineEdit *tempDir_; QComboBox *fileExistsPolicy_; QCheckBox *createSubfolderPerSite_; + QPlainTextEdit *allowedRoots_; // Connection QComboBox *connectionPreset_; diff --git a/gui/tests/CMakeLists.txt b/gui/tests/CMakeLists.txt index 221ce58..2139983 100644 --- a/gui/tests/CMakeLists.txt +++ b/gui/tests/CMakeLists.txt @@ -82,12 +82,14 @@ set_tests_properties(gui_fileinfodialog PROPERTIES LABELS "gui" ENVIRONMENT "QT_QPA_PLATFORM=offscreen") -# tst_optionsdialog — OptionsDialog::diffChanged, the "only send what changed" logic. -# Red when an unchanged key gets resent, or a key missing from `original` stops -# counting as changed. +# tst_optionsdialog — OptionsDialog::diffChanged, the "only send what changed" logic, and +# allKeys() against the real schema. +# Red when an unchanged key gets resent, a key missing from `original` stops counting +# as changed, or allKeys() drifts from Settings.schema.json in either direction. add_executable(tst_optionsdialog tst_optionsdialog.cpp) target_compile_features(tst_optionsdialog PRIVATE cxx_std_23) target_compile_options(tst_optionsdialog PRIVATE -Wall -Wextra -Wpedantic -Werror) +target_compile_definitions(tst_optionsdialog PRIVATE VELOX_REPO_ROOT="${CMAKE_SOURCE_DIR}") target_link_libraries(tst_optionsdialog PRIVATE velox-gui-lib Qt6::Widgets Qt6::Test) add_test(NAME gui_optionsdialog COMMAND tst_optionsdialog) set_tests_properties(gui_optionsdialog PROPERTIES diff --git a/gui/tests/tst_optionsdialog.cpp b/gui/tests/tst_optionsdialog.cpp index c5d2684..60fc782 100644 --- a/gui/tests/tst_optionsdialog.cpp +++ b/gui/tests/tst_optionsdialog.cpp @@ -5,6 +5,8 @@ // effect" contract and spams event.settings.changed with noise), or a key present in // `current` but absent from `original` stops being treated as changed. +#include +#include #include #include @@ -20,6 +22,7 @@ class TstOptionsDialog : public QObject { void onlyChangedKeysAreReturned(); void keyAbsentFromOriginalCountsAsChanged(); void allKeysAreNonEmptyAndUnique(); + void allKeysMatchesTheSchemaExactly(); }; void TstOptionsDialog::identicalValuesProduceNoDiff() { @@ -50,5 +53,24 @@ void TstOptionsDialog::allKeysAreNonEmptyAndUnique() { QCOMPARE(QSet(keys.begin(), keys.end()).size(), keys.size()); } +// Red when a key is added to (or removed from) Settings.schema.json without the same +// change landing here — either direction is a real bug: an invented key settings.set +// would reject with -32602, or a real key the dialog silently never shows. +void TstOptionsDialog::allKeysMatchesTheSchemaExactly() { + QFile f(QStringLiteral(VELOX_REPO_ROOT "/contracts/schema/types/Settings.schema.json")); + QVERIFY2(f.open(QIODevice::ReadOnly), qUtf8Printable(f.errorString())); + const QJsonObject schema = QJsonDocument::fromJson(f.readAll()).object(); + const QJsonObject properties = schema.value("properties").toObject(); + QVERIFY(!properties.isEmpty()); + + QSet schemaKeys; + for (auto it = properties.constBegin(); it != properties.constEnd(); ++it) { + schemaKeys.insert(it.key()); + } + const QStringList dialogKeysList = OptionsDialog::allKeys(); + const QSet dialogKeys(dialogKeysList.begin(), dialogKeysList.end()); + QCOMPARE(dialogKeys, schemaKeys); +} + QTEST_MAIN(TstOptionsDialog) #include "tst_optionsdialog.moc"