-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
FEAT(Client): Add Documentation dialog with official resource links #7144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -145,6 +145,8 @@ set(MUMBLE_SOURCES | |
| "Database.h" | ||
| "DeveloperConsole.cpp" | ||
| "DeveloperConsole.h" | ||
| "Documentation.cpp" | ||
| "Documentation.h" | ||
|
Comment on lines
+148
to
+149
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file names should have |
||
| "EchoCancelOption.cpp" | ||
| "EchoCancelOption.h" | ||
| "EnumStringConversions.cpp" | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the benefit of creating a dedicated dialog class for this rather than using a plain
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, dedicated class isn't really necessary. I can replace it with a plain QDialog set up inline at the call site. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // Copyright The Mumble Developers. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license | ||
| // that can be found in the LICENSE file at the root of the | ||
| // Mumble source tree or at <https://www.mumble.info/LICENSE>. | ||
|
|
||
| #include "Documentation.h" | ||
|
|
||
| #include <QtWidgets/QPushButton> | ||
| #include <QtWidgets/QTextBrowser> | ||
| #include <QtWidgets/QVBoxLayout> | ||
|
|
||
| DocumentationDialog::DocumentationDialog(QWidget *parent) : QDialog(parent) { | ||
| setWindowTitle(tr("Mumble Documentation")); | ||
| resize(640, 520); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hard-coded sizes are (almost) never a good idea. They will almost certainly break with certain system configurations. |
||
|
|
||
| QVBoxLayout *mainLayout = new QVBoxLayout(this); | ||
|
|
||
| QTextBrowser *documentationView = new QTextBrowser(this); | ||
| documentationView->setReadOnly(true); | ||
| documentationView->setOpenExternalLinks(true); | ||
| documentationView->setAccessibleName(tr("Documentation links")); | ||
| documentationView->setHtml( | ||
| tr("<h2>Welcome to Mumble Documentation</h2>" | ||
| "<h3>Documentation</h3>" | ||
| "<p>You can find official Mumble documentation and support resources below.</p>" | ||
| "<p>For new users, start with the <a href=\"https://www.mumble.info/documentation/user/\">User Guide</a>.</p>" | ||
| "<p>Configure key bindings with <a href=\"https://www.mumble.info/documentation/user/global-shortcuts/\">Global Shortcuts</a>.</p>" | ||
| "<p>Tune your setup with the <a href=\"https://www.mumble.info/documentation/user/audio-settings/\">Audio Settings guide</a>.</p>" | ||
| "<p>Browse all topics in <a href=\"https://www.mumble.info/documentation/\">Main Documentation</a>.</p>" | ||
| "<h3>Help and Resources</h3>" | ||
| "<p>Download the latest release from <a href=\"https://www.mumble.info/downloads/\">Downloads</a>.</p>" | ||
| "<p>Report issues or get technical help via <a href=\"https://github.com/mumble-voip/mumble/issues\">GitHub Issues</a>.</p>" | ||
| "<p>Read project updates on the <a href=\"https://www.mumble.info/blog/\">Blog</a>.</p>" | ||
| "<p>Reach the team through the <a href=\"https://www.mumble.info/contact/\">Contact page</a>.</p>")); | ||
|
Comment on lines
+20
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify URL-opening patterns and whether scheme validation is applied consistently.
# Expected: this new dialog currently appears with setOpenExternalLinks(true), while
# other call sites show explicit scheme checks.
rg -n -C2 'setOpenExternalLinks\s*\(\s*true\s*\)' --type=cpp
rg -n -C3 'QDesktopServices::openUrl\s*\(' --type=cpp
rg -n -C3 'allowedSchemes|url\.scheme\(\)\s*(!=|==)|contains\s*\(\s*url\.scheme\(\)\s*\)' --type=cppRepository: mumble-voip/mumble Length of output: 12666 Harden external-link handling with a scheme allowlist.
Note: Proposed fix `#include` "Documentation.h"
+#include <QtCore/QSet>
+#include <QtGui/QDesktopServices>
`#include` <QtWidgets/QPushButton>
`#include` <QtWidgets/QTextBrowser>
`#include` <QtWidgets/QVBoxLayout>
DocumentationDialog::DocumentationDialog(QWidget *parent) : QDialog(parent) {
@@
QTextBrowser *documentationView = new QTextBrowser(this);
documentationView->setReadOnly(true);
- documentationView->setOpenExternalLinks(true);
+ documentationView->setOpenExternalLinks(false);
+ connect(documentationView, &QTextBrowser::anchorClicked, this, [](const QUrl &url) {
+ static const QSet< QString > allowedSchemes = {
+ QStringLiteral("http"),
+ QStringLiteral("https"),
+ };
+
+ if (!url.isRelative() && allowedSchemes.contains(url.scheme())) {
+ QDesktopServices::openUrl(url);
+ }
+ });
documentationView->setAccessibleName(tr("Documentation links"));🤖 Prompt for AI Agents |
||
|
|
||
| QPushButton *okButton = new QPushButton(tr("OK"), this); | ||
| connect(okButton, SIGNAL(clicked()), this, SLOT(accept())); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use the type-safe overload that takes function pointers and doesn't require the |
||
|
|
||
| mainLayout->addWidget(documentationView); | ||
| mainLayout->addWidget(okButton); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Copyright The Mumble Developers. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license | ||
| // that can be found in the LICENSE file at the root of the | ||
| // Mumble source tree or at <https://www.mumble.info/LICENSE>. | ||
|
|
||
| #ifndef MUMBLE_MUMBLE_DOCUMENTATION_H_ | ||
| #define MUMBLE_MUMBLE_DOCUMENTATION_H_ | ||
|
|
||
| #include <QtWidgets/QDialog> | ||
|
|
||
| class DocumentationDialog : public QDialog { | ||
| private: | ||
| Q_OBJECT | ||
| Q_DISABLE_COPY(DocumentationDialog) | ||
|
|
||
| public: | ||
| explicit DocumentationDialog(QWidget *parent = nullptr); | ||
| }; | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be part of any commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, I'll remove it.