BUG: 385151
Use a KIO::copy job to ensure that ~/.face gets created correctly, and delete any temp files once they're no longer needed.
Details
Details
- Reviewers
apol mlaurent davidedmundson - Group Reviewers
Plasma - Commits
- R128:472f9a7446da: Fix using avatars from the gallery and from local files
Tested in KDE Neon. With this change, ~/.face gets created properly, both when it did not previously exist (i.e. when the user is setting a picture for the first time), and also when it did previously exist (i.e. when the user is changing their picture). Tested for gallery avatars as well as several local files (.png, .jpg, .webp).
Diff Detail
Diff Detail
- Repository
- R128 User Manager
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Please check we're not now leaking a file in /tmp when the user uses the pixmap region selector with a custom image.
Comment Actions
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.
Comment Actions
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.
Comment Actions
Not really. "Kcmshell5 usermanager" will still then leak.
Do you have any other ideas for solutions?
Comment Actions
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)
Comment Actions
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 .face directly)
| src/accountinfo.cpp | ||
|---|---|---|
| 458–462 | I know this isn't a line you've changed, and I hate it when reviewers comment about something unrelated, but it's on the screen and annoying me. model->setData( some qstring) how is that ever going to work? | |
Comment Actions
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.