-
-
Notifications
You must be signed in to change notification settings - Fork 458
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
cleanup IncognitoBrowser.cpp and support any firefox variant starting with "firefox-" #5788
Changes from 3 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 | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,15 +7,61 @@ | |||||||||||||||||
|
||||||||||||||||||
#include <QProcess> | ||||||||||||||||||
#include <QVariant> | ||||||||||||||||||
#include <QFileInfo> | ||||||||||||||||||
|
||||||||||||||||||
namespace chatterino { | ||||||||||||||||||
|
||||||||||||||||||
namespace { | ||||||||||||||||||
|
||||||||||||||||||
using namespace chatterino; | ||||||||||||||||||
constexpr bool isWindows() | ||||||||||||||||||
{ | ||||||||||||||||||
#if Q_OS_WINDOWS | ||||||||||||||||||
return true; | ||||||||||||||||||
#else | ||||||||||||||||||
return false; | ||||||||||||||||||
#endif | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
QString getExecutable() | ||||||||||||||||||
{ | ||||||||||||||||||
#ifdef USEWINSDK | ||||||||||||||||||
|
||||||||||||||||||
using query = getAssociatedExecutable; | ||||||||||||||||||
using QueryType = AssociationQueryType; | ||||||||||||||||||
|
||||||||||||||||||
QString cmd; | ||||||||||||||||||
|
||||||||||||||||||
if ((cmd = query(QueryType::Protocol, L"HTTP"))) | ||||||||||||||||||
return cmd; | ||||||||||||||||||
if ((cmd = query(QueryType::FileExtension, L".html"))) | ||||||||||||||||||
return cmd; | ||||||||||||||||||
if ((cmd = query(QueryType::FileExtension, L".htm"))) | ||||||||||||||||||
return cmd; | ||||||||||||||||||
|
||||||||||||||||||
#elif defined(Q_OS_UNIX) && !defined(Q_OS_DARWIN) | ||||||||||||||||||
|
||||||||||||||||||
auto desktopFile = getDefaultBrowserDesktopFile(); | ||||||||||||||||||
|
||||||||||||||||||
if (desktopFile.has_value()) | ||||||||||||||||||
{ | ||||||||||||||||||
auto entries = desktopFile->getEntries("Desktop Entry"); | ||||||||||||||||||
|
||||||||||||||||||
auto exec = entries.find("Exec"); | ||||||||||||||||||
if (exec != entries.end()) | ||||||||||||||||||
return parseDesktopExecProgram(exec->second.trimmed()); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
#endif | ||||||||||||||||||
|
||||||||||||||||||
// no browser found or platform not supported | ||||||||||||||||||
return {}; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
QString getPrivateSwitch(const QString &browserExecutable) | ||||||||||||||||||
QString getPrivateArg(const QString &browserPath) | ||||||||||||||||||
{ | ||||||||||||||||||
// list of command line switches to turn on private browsing in browsers | ||||||||||||||||||
static auto switches = std::vector<std::pair<QString, QString>>{ | ||||||||||||||||||
// list of command line arguments to turn on private browsing | ||||||||||||||||||
static std::vector<std::pair<QString, QString>> argTable = {{ | ||||||||||||||||||
{"firefox", "-private-window"}, | ||||||||||||||||||
{"librewolf", "-private-window"}, | ||||||||||||||||||
{"waterfox", "-private-window"}, | ||||||||||||||||||
|
@@ -27,95 +73,50 @@ QString getPrivateSwitch(const QString &browserExecutable) | |||||||||||||||||
{"opera\\launcher", "--private"}, | ||||||||||||||||||
{"iexplore", "-private"}, | ||||||||||||||||||
{"msedge", "-inprivate"}, | ||||||||||||||||||
{"firefox-esr", "-private-window"}, | ||||||||||||||||||
{"chromium", "-incognito"}, | ||||||||||||||||||
{"brave", "-incognito"}, | ||||||||||||||||||
{"firefox-devedition", "-private-window"}, | ||||||||||||||||||
{"firefox-developer-edition", "-private-window"}, | ||||||||||||||||||
{"firefox-beta", "-private-window"}, | ||||||||||||||||||
{"firefox-nightly", "-private-window"}, | ||||||||||||||||||
}; | ||||||||||||||||||
}}; | ||||||||||||||||||
|
||||||||||||||||||
// compare case-insensitively | ||||||||||||||||||
auto lowercasedBrowserExecutable = browserExecutable.toLower(); | ||||||||||||||||||
// may return null string if path is '/' | ||||||||||||||||||
QString exeFilename = QFileInfo(browserPath).fileName(); | ||||||||||||||||||
teknsl marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
|
||||||||||||||||||
#ifdef Q_OS_WINDOWS | ||||||||||||||||||
if (lowercasedBrowserExecutable.endsWith(".exe")) | ||||||||||||||||||
if (!exeFilename.isNull()) | ||||||||||||||||||
{ | ||||||||||||||||||
lowercasedBrowserExecutable.chop(4); | ||||||||||||||||||
} | ||||||||||||||||||
#endif | ||||||||||||||||||
|
||||||||||||||||||
for (const auto &switch_ : switches) | ||||||||||||||||||
{ | ||||||||||||||||||
if (lowercasedBrowserExecutable.endsWith(switch_.first)) | ||||||||||||||||||
{ | ||||||||||||||||||
return switch_.second; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// couldn't match any browser -> unknown browser | ||||||||||||||||||
return {}; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
QString getDefaultBrowserExecutable() | ||||||||||||||||||
{ | ||||||||||||||||||
#ifdef USEWINSDK | ||||||||||||||||||
// get default browser start command, by protocol if possible, falling back to extension if not | ||||||||||||||||||
QString command = | ||||||||||||||||||
getAssociatedExecutable(AssociationQueryType::Protocol, L"http"); | ||||||||||||||||||
|
||||||||||||||||||
if (command.isNull()) | ||||||||||||||||||
{ | ||||||||||||||||||
// failed to fetch default browser by protocol, try by file extension instead | ||||||||||||||||||
command = getAssociatedExecutable(AssociationQueryType::FileExtension, | ||||||||||||||||||
L".html"); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if (command.isNull()) | ||||||||||||||||||
{ | ||||||||||||||||||
// also try the equivalent .htm extension | ||||||||||||||||||
command = getAssociatedExecutable(AssociationQueryType::FileExtension, | ||||||||||||||||||
L".htm"); | ||||||||||||||||||
// mozilla distributes many firefox variants with different endings so | ||||||||||||||||||
// catch them all here, and be future proof at the same time :) | ||||||||||||||||||
if (exeFilename.startsWith("firefox-")) | ||||||||||||||||||
return "-private-window"; | ||||||||||||||||||
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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
for (const auto &pair : argTable) | ||||||||||||||||||
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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
src/util/IncognitoBrowser.cpp:87: - return pair.second;
+ return pair.second;
+ } 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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
src/util/IncognitoBrowser.cpp:91: - return pair.second;
+ return pair.second;
+ } 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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
src/util/IncognitoBrowser.cpp:90: - return pair.second;
+ return pair.second;
+ } 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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
src/util/IncognitoBrowser.cpp:89: - return pair.second;
+ return pair.second;
+ } 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. i disagree with this but can get changed if need be |
||||||||||||||||||
if (exeFilename == pair.first) | ||||||||||||||||||
return pair.second; | ||||||||||||||||||
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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return command; | ||||||||||||||||||
#elif defined(Q_OS_UNIX) && !defined(Q_OS_DARWIN) | ||||||||||||||||||
static QString defaultBrowser = []() -> QString { | ||||||||||||||||||
auto desktopFile = getDefaultBrowserDesktopFile(); | ||||||||||||||||||
if (desktopFile.has_value()) | ||||||||||||||||||
{ | ||||||||||||||||||
auto entry = desktopFile->getEntries("Desktop Entry"); | ||||||||||||||||||
auto exec = entry.find("Exec"); | ||||||||||||||||||
if (exec != entry.end()) | ||||||||||||||||||
{ | ||||||||||||||||||
return parseDesktopExecProgram(exec->second.trimmed()); | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
return {}; | ||||||||||||||||||
}(); | ||||||||||||||||||
|
||||||||||||||||||
return defaultBrowser; | ||||||||||||||||||
#else | ||||||||||||||||||
// unsupported or invalid browser | ||||||||||||||||||
return {}; | ||||||||||||||||||
#endif | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
} // namespace | ||||||||||||||||||
|
||||||||||||||||||
namespace chatterino { | ||||||||||||||||||
|
||||||||||||||||||
bool supportsIncognitoLinks() | ||||||||||||||||||
{ | ||||||||||||||||||
auto browserExe = getDefaultBrowserExecutable(); | ||||||||||||||||||
return !browserExe.isNull() && !getPrivateSwitch(browserExe).isNull(); | ||||||||||||||||||
QString browserExe = getExecutable(); | ||||||||||||||||||
if (browserExe.isNull()) | ||||||||||||||||||
return false; | ||||||||||||||||||
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. warning: statement should be inside braces [readability-braces-around-statements]
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
QString browserArg = getPrivateArg(browserExe); | ||||||||||||||||||
if (browserArg.isNull()) | ||||||||||||||||||
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. warning: statement should be inside braces [readability-braces-around-statements] if (browserArg.isNull())
^ this fix will not be applied because it overlaps with another fix |
||||||||||||||||||
return false; | ||||||||||||||||||
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. warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] src/util/IncognitoBrowser.cpp:101: - if (browserArg.isNull())
- return false;
-
- return true;
+ return !browserArg.isNull(); 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. warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] src/util/IncognitoBrowser.cpp:107: - if (browserArg.isNull())
- return false;
-
- return true;
+ return !browserArg.isNull(); 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. warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr] src/util/IncognitoBrowser.cpp:108: - if (browserArg.isNull())
- return false;
-
- return true;
+ return !browserArg.isNull(); |
||||||||||||||||||
|
||||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
bool openLinkIncognito(const QString &link) | ||||||||||||||||||
{ | ||||||||||||||||||
auto browserExe = getDefaultBrowserExecutable(); | ||||||||||||||||||
return QProcess::startDetached(browserExe, | ||||||||||||||||||
{getPrivateSwitch(browserExe), link}); | ||||||||||||||||||
QString browserExe = getExecutable(); | ||||||||||||||||||
QString browserArg = getPrivateArg(browserExe); | ||||||||||||||||||
|
||||||||||||||||||
return QProcess::startDetached(browserExe, {browserArg, link}); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
} // namespace chatterino |
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.
warning: statement should be inside braces [readability-braces-around-statements]