Remove KUrl from autorefresh, dirfilter, domtreeview and fsview plugins
ClosedPublic

Authored by stefanocrocco on Aug 21 2018, 7:03 PM.

Details

Summary

Remove use of KUrl from plugins

This is the first step in the long road to remove use of KDELibs4Support

Plugins which currently aren't compiled (for example, UAChanger or
AdBlock) haven't been changed

Test Plan

check that konqueror and its plugins compile

Diff Detail

Repository
R226 Konqueror
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
stefanocrocco requested review of this revision.Aug 21 2018, 7:03 PM
stefanocrocco created this revision.
dfaure requested changes to this revision.Aug 22 2018, 8:20 AM
dfaure added inline comments.
plugins/fsview/fsview.cpp
164

This will lead to a URL without scheme, which is not really useful.

This should be QUrl u = QUrl::fromLocalFile(....);

It's the biggest porting trap from KUrl to QUrl.

plugins/fsview/fsview_part.cpp
350

fromLocalFile

388

same

552

same

plugins/fsview/inode.cpp
424

same

plugins/fsview/scan.cpp
270โ€“271

same

plugins/kimgalleryplugin/imgallerydialog.cpp
459

This should probably be .toLocalFile(), which makes a difference on Windows.

plugins/kimgalleryplugin/imgalleryplugin.cpp
55

add static in front

(add space before '{' ?)

56

You could combine both flags in a single adjusted() call, no?

Are you sure about the FullyDecoded? That usually gives trouble.

285

clearly directory() must return a local path then, not a fully decoded URL (with file scheme in front).

I think you need toLocalFile() in directory().

347โ€“351

toLocalFile()

plugins/rellinks/plugin_rellinks.cpp
54

static in front, & after QString

528

KUrl(base, end) didn't return a relative URL. On the contrary, it would resolve end against base if end is relative.

On the next line the URL is passed to openUrlRequest, which only supports absolute URLs.

plugins/webarchiver/archivedialog.cpp
35

revert ;)

936

Are we sure baseurl has no fragment itself? The old code would remove it from both.

1376

suburls are not supported anymore, they were a KDE3 thing.

OK except for error:/, in a way, that's still used.
But kioslaves can't be chained with suburls.

This revision now requires changes to proceed.Aug 22 2018, 8:20 AM
stefanocrocco added inline comments.Aug 22 2018, 9:27 AM
plugins/kimgalleryplugin/imgalleryplugin.cpp
56

According to the documentation for KUrl::directory, the two calls to QUrl::adjusted shouldn't be combined. Indeed, the results are different:

QUrl("file://abc/xyz/").adjusted(QUrl::RemoveTrailingSlash).adjusted(QUrl::RemoveFilename).toDisplayString()

gives file://xyz, while

QUrl("file://abc/xyz/").adjusted(QUrl::RemoveTrailingSlash|QUrl::RemoveFilename).toDisplayString()

gives file://xyz/abc.

plugins/webarchiver/archivedialog.cpp
936

According to the documentation for QUrl::matches, the given options are applied to both URLs before coomparing them, so there should be no problems.

dfaure added inline comments.Aug 22 2018, 9:29 AM
plugins/kimgalleryplugin/imgalleryplugin.cpp
56

Ah OK, thanks for the explanation.

plugins/webarchiver/archivedialog.cpp
936

Indeed. Sorry, I read too quickly.

Made the requested changes

dfaure requested changes to this revision.Aug 23 2018, 10:35 PM
dfaure added inline comments.
plugins/kimgalleryplugin/imgalleryplugin.cpp
368

QUrl::fromLocalFile here as well

This revision now requires changes to proceed.Aug 23 2018, 10:35 PM

Replace QUrl() with QUrl::fromLocalFile()

dfaure accepted this revision.Aug 24 2018, 7:18 AM
This revision is now accepted and ready to land.Aug 24 2018, 7:18 AM
This revision was automatically updated to reflect the committed changes.