Differential D17372 Diff 46914 kcms/componentchooser/componentchooserbrowser.cpp

Changeset View

Standalone View

View Options

kcms/componentchooser/componentchooserbrowser.cpp

Show All 25 Lines
26 26​
27#include <QUrl> 27​#include <QUrl>
28 28​
29CfgBrowser::CfgBrowser(QWidget *parent) 29​CfgBrowser::CfgBrowser(QWidget *parent)
30 : QWidget(parent), Ui::BrowserConfig_UI(),CfgPlugin() 30​ : QWidget(parent), Ui::BrowserConfig_UI(),CfgPlugin()
31{ 31​{
32 setupUi(this); 32​ setupUi(this);
33 connect(lineExec, &KLineEdit::textChanged, this, &CfgBrowser::configChanged); 33​ connect(lineExec, &KLineEdit::textChanged, this, &CfgBrowser::configChanged);
34 connect(radioKIO, &QRadioButton::toggled, this, &CfgBrowser::configChanged); ​
35 connect(radioService, &QRadioButton::toggled, this, &CfgBrowser::selectBrowserApp); 34​ connect(radioService, &QRadioButton::toggled, this, &CfgBrowser::selectBrowserApp);
36 connect(browserCombo, static_cast<void(QComboBox::*)(int)>(&QComboBox::activated), this, &CfgBrowser::selectBrowserApp); 35​ connect(browserCombo, static_cast<void(QComboBox::*)(int)>(&QComboBox::activated), this, &CfgBrowser::selectBrowserApp);
37 connect(radioExec, &QRadioButton::toggled, this, &CfgBrowser::configChanged); 36​ connect(radioExec, &QRadioButton::toggled, this, &CfgBrowser::configChanged);
38 connect(btnSelectApplication, &QToolButton::clicked, this, &CfgBrowser::selectBrowser); 37​ connect(btnSelectApplication, &QToolButton::clicked, this, &CfgBrowser::selectBrowser);
39} 38​}
40 39​
41CfgBrowser::~CfgBrowser() { 40​CfgBrowser::~CfgBrowser() {
42} 41​}
Show All 13 Lines
56 55​
57void CfgBrowser::defaults() 56​void CfgBrowser::defaults()
58{ 57​{
59 load(nullptr); 58​ load(nullptr);
60} 59​}
61 60​
62void CfgBrowser::load(KConfig *) 61​void CfgBrowser::load(KConfig *)
63{ 62​{
63​ browserCombo->clear();
64​ const auto &browsers = KServiceTypeTrader::self()->query(QStringLiteral("Application"),
65​ QStringLiteral("'WebBrowser' in Categories"));
66​ for (const auto &service : browsers) {
67​ browserCombo->addItem(QIcon::fromTheme(service->icon()), service->name(), service->storageId());
68​
69​ if ((m_browserService && m_browserService->storageId() == service->storageId()) || service->exec() == m_browserExec) {
davidedmundsonUnsubmitted
Done

m_browserService is initialised after this.

It won't set the correct index.

davidedmundson: m_browserService is initialised after this. It won't set the correct index.
70​ browserCombo->setCurrentIndex(browserCombo->count() - 1);
71​ radioService->setChecked(true);
72​ }
73​ }
74​
64 const KConfigGroup config(KSharedConfig::openConfig(QStringLiteral("kdeglobals")), QStringLiteral("General") ); 75​ const KConfigGroup config(KSharedConfig::openConfig(QStringLiteral("kdeglobals")), QStringLiteral("General") );
65 const QString exec = config.readPathEntry( QStringLiteral("BrowserApplication"), QString() ); 76​ const QString exec = config.readPathEntry( QStringLiteral("BrowserApplication"), QString() );
66 if (exec.isEmpty()) { 77​ if (exec.isEmpty()) {
67 radioKIO->setChecked(true); 78​ radioService->setChecked(true);
68 m_browserExec = exec; 79​ if (browserCombo->count() > 0) {
69 m_browserService = nullptr; 80​ const QString &storageId = browserCombo->currentData().toString();
dfaureUnsubmitted
Not Done

This will lead to rather confusing user experience, btw.

On a freshly installed system [without distro hacks], BrowserApplication is empty.
In practice that means KRun::setEnableExternalBrowser will pick the entry x-scheme-handler/https in the user's mimeapps.list, if any. And if that isn't set either, then well, the current behaviour in KRun, AFAICS, *will* be to use KIO to find out the mimetype.

Then the user opens this KCM, and it says "your default browser is firefox" (or whichever is first in the list). "What?" says the user... "that's not what I'm experiencing..."

If you want to change the default behaviour, you need to actually change the default behaviour (in KRun), not just what the KCM shows by default.

The default behaviour of KRun and the default settings shown by this KCM have to match.

This investigation shows that this is already buggy before this patch (the fallback to x-scheme-handler/https in the user's mimeapps.list is missing here), but this patch makes it much worse, as is.

dfaure: This will lead to rather confusing user experience, btw. On a freshly installed system…
81​ m_browserService = KService::serviceByStorageId(storageId);
82​ m_browserExec.clear();
83​ }
70 } else { 84​ } else {
71 radioExec->setChecked(true); 85​ radioExec->setChecked(true);
72 if (exec.startsWith('!')) { 86​ if (exec.startsWith('!')) {
73 m_browserExec = exec.mid(1); 87​ m_browserExec = exec.mid(1);
74 m_browserService = nullptr; 88​ m_browserService = nullptr;
75 } else { 89​ } else {
76 m_browserService = KService::serviceByStorageId( exec ); 90​ m_browserService = KService::serviceByStorageId( exec );
77 if (m_browserService) { 91​ if (m_browserService) {
78 m_browserExec = m_browserService->desktopEntryName(); 92​ m_browserExec = m_browserService->desktopEntryName();
79 } else { 93​ } else {
80 m_browserExec.clear(); 94​ m_browserExec.clear();
81 } 95​ }
82 } 96​ }
83 } 97​ }
84 98​
85 lineExec->setText(m_browserExec); 99​ lineExec->setText(m_browserExec);
86 100​
87 browserCombo->clear(); ​
88 ​
89 const auto &browsers = KServiceTypeTrader::self()->query(QStringLiteral("Application"), ​
90 QStringLiteral("'WebBrowser' in Categories")); ​
91 for (const auto &service : browsers) { ​
92 browserCombo->addItem(QIcon::fromTheme(service->icon()), service->name(), service->storageId()); ​
93 ​
94 if ((m_browserService && m_browserService->storageId() == service->storageId()) || service->exec() == m_browserExec) { ​
95 browserCombo->setCurrentIndex(browserCombo->count() - 1); ​
96 radioService->setChecked(true); ​
97 } ​
98 } ​
99 ​
100 emit changed(false); 101​ emit changed(false);
101} 102​}
102 103​
103void CfgBrowser::save(KConfig *) 104​void CfgBrowser::save(KConfig *)
104{ 105​{
105 KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("kdeglobals")); 106​ KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("kdeglobals"));
106 KConfigGroup config(profile, QStringLiteral("General")); 107​ KConfigGroup config(profile, QStringLiteral("General"));
107 QString exec; 108​ QString exec;
▲ Show 20 Lines • Show All 63 Lines • Show Last 20 Lines