Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog
AcceptedPublic

Authored by dfaure on Sat, May 9, 11:01 AM.

Details

Summary

This will not look up the preferred app or run scripts/executables, it always shows an open-with dialog.
Bonus feature compared to KRun::displayOpenWithDialog or KOpenWithDialog: it will first determine the mimetype
(even for remote URLs), which allows to "remember this app for this mimetype".

On the other hand, this loses the support for multiple URLs; OpenUrlJob is about a single URL.
I guess for multiple URLs the best solution is to use KOpenWithDialog directly.

Test Plan

New unittest

Diff Detail

Repository
R241 KIO
Branch
2020_05_displayOpenWithDialog
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26987
Build 27005: arc lint + arc unit
dfaure created this revision.Sat, May 9, 11:01 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptSat, May 9, 11:01 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
dfaure requested review of this revision.Sat, May 9, 11:01 AM
ahmadsamir accepted this revision.Fri, May 15, 4:59 PM
ahmadsamir added a subscriber: kossebau.
ahmadsamir added inline comments.
src/gui/openurljob.h
121

It seems that we shouldn't end @param with a ".", according to @kossebau anyway...

This revision is now accepted and ready to land.Fri, May 15, 4:59 PM
kossebau added inline comments.Fri, May 15, 5:09 PM
src/gui/openurljob.h
120

Please mention explicit what the default is (false), to remove any ambiguity.

Some other setters might want to have this stated as well, btw.

ahmadsamir added inline comments.Fri, May 15, 5:13 PM
src/gui/openurljob.h
121

I got the info from a commit in kwidgetsaddons where you "fixed" a previous commit of mine :)

Thanks for the link though.

dfaure updated this revision to Diff 82992.Sat, May 16, 9:05 AM
dfaure marked 2 inline comments as done.

API docs: remove trailing dot, mention 2 default values

dfaure closed this revision.Sat, May 16, 9:32 AM

OK I'm having second thoughts about this. Because of Windows, and because of the case of multiple URLs.

There's "displaying an open with dialog because we couldn't find any app, after a left-click on a file"
and there's "displaying an open with dialog because the user explicitly clicked on Open With..."

On Windows, it's OK if the first one falls back to QDesktopServices::openUrl (i.e. actually launch an app, no open with dialog)
while the second case must show a dialog (KRun::displayOpenWithDialog had code for the Windows native open-with dialog).

Also OpenUrlJob can only ever support a single URL while displayOpenWithDialog supports multiple URLs.

Maybe I should move all this over to ApplicationLauncherJob, which supports multiple URLs (adding a ctor without a KService). Or to yet another job class...
We lose mimetype determination... but that's what we didn't have in KRun::displayOpenWithDialog either.

Input welcome...

dfaure reopened this revision.Sun, May 17, 9:41 AM

Not committed after all.

This revision is now accepted and ready to land.Sun, May 17, 9:41 AM

Not committed after all.

It was committed, as I had it when I did git pull, but you lucked out, it seemed to have been eaten by the migration to gitlab somehow; I had to 'git reset --hard' to be in sync with invent.kde.org :)

Yeah, the full story is that I pushed to git.kde.org, reverted because I changed my mind, then the sysadmins told me I wasn't supposed to push to git.kde.org at all (it was blocked for everyone, but I have sysadmin privileges for historical and bus-factor reasons). We realized there was nothing to sync to gitlab anyway, since I had simply done push+push+revert+revert.

I think I want to add ApplicationLauncherJob(/*no service arg*/) and make it pop up the open with dialog and launch the selected application.