D8793: Fix using avatars from the gallery and from local files

2017-11-15 Thread Nathaniel Graham
This revision was automatically updated to reflect the committed changes. Closed by commit R128:472f9a7446da: Fix using avatars from the gallery and from local files (authored by ngraham). REPOSITORY R128 User Manager CHANGES SINCE LAST UPDATE

D8793: Fix using avatars from the gallery and from local files

2017-11-15 Thread Nathaniel Graham
ngraham added a comment. Thanks for the approval. I'll land this, then the refactoring onto my to-do list. I agree that the current code is a bit stinky and could benefit from a more direct approach. REPOSITORY R128 User Manager BRANCH arcpatch-D8793_2 REVISION DETAIL

D8793: Fix using avatars from the gallery and from local files

2017-11-15 Thread David Edmundson
davidedmundson accepted this revision. davidedmundson added a comment. No objections. It fixes a bug... But, if you have the time, I'm sure we could come up wtih something much more elegant with some refactoring. (like storing the pixmap in infToSave, and then just writing it into

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread Nathaniel Graham
ngraham removed a subscriber: Dolphin. REPOSITORY R128 User Manager BRANCH arcpatch-D8793_2 REVISION DETAIL https://phabricator.kde.org/D8793 To: ngraham, apol, #plasma, mlaurent, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread Nathaniel Graham
ngraham updated this revision to Diff 22377. ngraham added a comment. Restore correct diff REPOSITORY R128 User Manager CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8793?vs=22375=22377 BRANCH arcpatch-D8793_2 REVISION DETAIL https://phabricator.kde.org/D8793 AFFECTED

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread Nathaniel Graham
ngraham updated this revision to Diff 22375. ngraham added a comment. Restricted Application added a subscriber: Dolphin. Rely on KF 5.40 and simplify the code a ton REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8793?vs=22374=22375 BRANCH

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread Nathaniel Graham
ngraham updated this revision to Diff 22374. ngraham added a comment. Consider any source file in /tmp/ to be a temp file that we should clear, so we don't leak files when run by kcmshell5 (or anything similar) REPOSITORY R128 User Manager CHANGES SINCE LAST UPDATE

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread David Edmundson
davidedmundson added a comment. Not really. "Kcmshell5 usermanager" will still then leak. Do you have any other ideas for solutions? REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D8793 To: ngraham, apol, #plasma, mlaurent, davidedmundson

D8793: Fix using avatars from the gallery and from local files

2017-11-14 Thread Nathaniel Graham
ngraham added a comment. @davidedmundson, does your approval still hold with the new change to clean up /tmp? REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D8793 To: ngraham, apol, #plasma, mlaurent, davidedmundson Cc: plasma-devel, ZrenBot,

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread Nathaniel Graham
ngraham edited the summary of this revision. REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D8793 To: ngraham, apol, #plasma, mlaurent, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas,

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread Nathaniel Graham
ngraham updated this revision to Diff 22306. ngraham added a comment. Found a way to safely remove the temp file and keep everything working REPOSITORY R128 User Manager CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8793?vs=22272=22306 BRANCH master REVISION DETAIL

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread Nathaniel Graham
ngraham added a comment. Actually, deleting the file in /tmp breaks it all over again in the same way, which is probably why this was broken when it was using a moveJob. Looks like we may have to live with having a file in /tmp, or else embark on a bigger change. REPOSITORY R128 User

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread Nathaniel Graham
ngraham added a comment. Yes, it looks like in that case, we leak /tmp/systemsettings.[number] since it's copied, not moved. I'll have to explicitly handle that case. REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D8793 To: ngraham, apol,

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread David Edmundson
davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Please check we're not now leaking a file in /tmp when the user uses the pixmap region selector with a custom image. REPOSITORY R128 User Manager BRANCH master

D8793: Fix using avatars from the gallery and from local files

2017-11-13 Thread Nathaniel Graham
ngraham retitled this revision from "Fix using avatars from the gallery" to "Fix using avatars from the gallery and from local files". ngraham edited the test plan for this revision. ngraham added reviewers: apol, Plasma, mlaurent, davidedmundson. REPOSITORY R128 User Manager REVISION DETAIL

D8793: Fix using avatars from the gallery

2017-11-13 Thread Nathaniel Graham
ngraham created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY BUG: 385151 Use a KIO::copy job throughout to ensure that ~/.face gets created correctly TEST PLAN Tested in KDE Neon. With this