[componentchooser KCM] Make KIO browser option the fallback only and remove from the UI
AbandonedPublic

Authored by ngraham on Dec 5 2018, 7:57 PM.

Details

Reviewers
dfaure
Group Reviewers
Plasma
VDG
Frameworks
Summary

Right now the componentchooser KCM's browser module has a really confusing setting ("In an application based on the contents of the url") that's the default. There are ancient Buzailla tickets about this being confusing, and some users report that it actually does the wrong thing for certain URLs.

This patch removes the UI for this feature and, in conjunction with D17371, makes it a hidden fallback-only behavior for the rare case that there aren't any web browsers installed.

BUG: 100016
FIXED-IN: 5.15.0

Test Plan

Here's what it looks like now:

Verifications performed:

  • There are no actual settings or behavioral changes for any existing users, since browsers ask to set themselves as the default, and distros ship config files already specifying their preferred browser.
  • Changing the browser in the combobox correctly updates both ~/.config/kdeglobals and ~/.configmimeapps.list.
  • The combobox's default setting is now correctly identical to what ~/.config/mimeapps.list specifies, if anything.

Diff Detail

Repository
R119 Plasma Desktop
Branch
arcpatch-D17372
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6508
Build 6526: arc lint + arc unit
ngraham created this revision.Dec 5 2018, 7:57 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 5 2018, 7:57 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Dec 5 2018, 7:57 PM
ngraham edited the summary of this revision. (Show Details)Dec 5 2018, 7:58 PM
ngraham edited the test plan for this revision. (Show Details)

and some users report that it actually does the wrong thing for certain URLs.

Bug reports are also an indication people use it.

I don't see a compelling reason to remove a feature.

and some users report that it actually does the wrong thing for certain URLs.

Bug reports are also an indication people use it.

Well, it's the default, so I think that goes without saying. :)

The reason for removing it is because it's not what people expect in the year 2018. When you open an HTTP or HTTPS link, you expect it to open in your browser, full stop.

abetts added a subscriber: abetts.Dec 11 2018, 3:51 PM

Can the section title be called something different instead of using the word "Component"?

dfaure added a subscriber: dfaure.Dec 22 2018, 11:24 PM

OK, here's a bit of historical information, and then my opinion.

The idea of that option was that when you click on a (e.g. HTTP) link to a PDF (e.g. in an email), the best application to view that PDF is Okular.
Similarly, if it's an image, the best application would be an image viewer.
Sure, those aren't browsers, that's the whole point of that option.
When the associated application is using KIO, there is no speed penalty because the job (and the kioslave) used to retrieve the URL is put on hold in the first app (e.g. kmail) and resumed in the application being launched.
I don't think this is confusing in any way (only be people who nitpick on the word "browser" in the section name), but that's a minor issue at this point, see below.

This used to be fine, at least with KDE applications. But that means a double HTTP request with non-KDE applications.
And these days there's no KIO-based browser anymore, so for the common case (HTML) it doesn't work anymore.

I agree to removing the option, then.
Even I don't use it anymore, even though it was one of my favourite features...
I'm just sad that clicking on a link to a PDF in kmail doesn't open okular but firefox's awful embedded PDF reader. But we can't trust extensions over HTTP, so there's not really any solution there. Thank you Apple, the killing of QtWebKit has killed support for this, indirectly.

If we are going to effectively remove it, I would like to see it removed fully.

Otherwise we'll get some crufty code that doesn't make sense forever as well as quirky bugs.

kcms/componentchooser/componentchooserbrowser.cpp
69

m_browserService is initialised after this.

It won't set the correct index.

ngraham edited the test plan for this revision. (Show Details)Dec 24 2018, 3:45 PM
ngraham updated this revision to Diff 48456.Dec 31 2018, 5:41 PM

Initialize m_browserCombo before accessing it

ngraham marked an inline comment as done.Dec 31 2018, 5:41 PM
ngraham updated this revision to Diff 48457.Dec 31 2018, 5:42 PM

Revert unintentional and unnecessary whitespace change

If folks really want, I can have a look at removing the feature entirely. However are we sure we don't want to keep it around as a fallback? It doesn't necessarily seem like a bad backup, it's just maybe not appropriate as a user-visible setting anymore in 2018.

I'm still not sure whether the feature needs to be removed entirely. I feel that this minimal patch probably stands on its own well enough as-is.

dfaure requested changes to this revision.May 31 2019, 5:49 PM

I agree that we shouldn't remove the underlying feature, in fact it's still used on other protocols than http.

But we have to ensure the default behaviour matches the KCM and vice-versa, for http.

kcms/componentchooser/componentchooserbrowser.cpp
68

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.

This revision now requires changes to proceed.May 31 2019, 5:49 PM
ngraham planned changes to this revision.May 31 2019, 6:37 PM

I don't think it's a "distro hack" to set a default browser, which is why they all do it. Not having a default browser set seems like an error condition to me. However I see what you meanabout the bugginess and will work on it.

Writing into the user's home dir is a "hack".

There are better ways for distros to set defaults (such as the global mimeapps.list) but it seems KIO ignores that... (to be checked...)

meven added a subscriber: meven.Jan 17 2020, 4:48 PM

Writing into the user's home dir is a "hack".

There are better ways for distros to set defaults (such as the global mimeapps.list) but it seems KIO ignores that... (to be checked...)

It did (did not read [Default Applications] x-scheme-handle/http basicaly.
I have a fix for that : D26690

And I have another go at this UI : D26731

ngraham abandoned this revision.Jan 17 2020, 4:49 PM

Abandoning in favor of those patches. Thanks @meven!