Differential D7192

Receiving a file with a '#' or '?' in the name from Android no longer results in the name cropped.
AbandonedPublic
Actions

Authored by jeanv on Aug 7 2017, 3:52 PM.

Details

Reviewers
apol
Group Reviewers
KDE Connect
Summary

Because '#' and '?' are valid URL characters, without QUrl::fromLocalFile, they don't get properly percent-encoded.
The problem was more visible with the number sign since Android is pretty adamant about preventing question marks in filenames.

See T6730.

Diff Detail

Repository
R224 KDE Connect
Lint
Lint Skipped
Unit
Unit Tests Skipped
jeanv created this revision.Aug 7 2017, 3:52 PM
jeanv added a reviewer: KDE Connect.
jeanv edited the summary of this revision. (Show Details)Aug 7 2017, 3:54 PM
apol added a subscriber: apol.Aug 9 2017, 8:50 PM

To me it sounds like the issue you are trying to solve is elsewhere.

Maybe make sure that in FileTransferJob::startTransfer() the destination is still correct?

plugins/share/shareplugin.cpp
92

RemoveScheme? Are you trying to do toLocalFile?

apol requested changes to this revision.Aug 9 2017, 8:52 PM
This revision now requires changes to proceed.Aug 9 2017, 8:52 PM
jeanv added a comment.Aug 10 2017, 6:39 AM
In D7192#134232, @apol wrote:

Please see https://phabricator.kde.org/D7224

I see what you mean. Your solution does seem better.

apol added a comment.Aug 10 2017, 10:30 AM
In D7192#134257, @jeanv wrote:
In D7192#134232, @apol wrote:

Please see https://phabricator.kde.org/D7224

I see what you mean. Your solution does seem better.

Then please, review my patch :)

jeanv added a comment.Aug 10 2017, 10:57 AM
In D7192#134274, @apol wrote:
In D7192#134257, @jeanv wrote:
In D7192#134232, @apol wrote:

Please see https://phabricator.kde.org/D7224

I see what you mean. Your solution does seem better.

Then please, review my patch :)

Oooh right :)

nicolasfella added a subscriber: nicolasfella.Aug 13 2017, 4:57 PM

I guess this can be closed now?

jeanv abandoned this revision.Aug 13 2017, 7:14 PM
In D7192#135236, @nicolasfella wrote:

I guess this can be closed now?

Absolutely

Revision Contents
Changeset List

PathPackages
Mplugins/share/shareplugin.cpp (6 lines)

Diff 17841

View Options

plugins/share/shareplugin.cpp

Loading...