D7170: Fix errorneous whitespace handling in Desktop Entry parsing from DesktopFileParser
arichardson added a comment. Looks good to me. Using a regex engine for this seems like overkill but I guess compared to the I/O overhead of reading the desktop file it shouldn't matter. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D7170 To: mpyne, #frameworks, arichardson Cc: apol
D6832: Integrate new file ioslave in KIO job
dfaure added inline comments. INLINE COMMENTS > simplejob.cpp:346 > +bool confirmed = tryAskPrivilegeOpConfirmation(); > +m_slave->send(MSG_PRIVILEGE_EXEC, QByteArray(confirmed ? "1" : "0")); > +} Check that this is consistent with SlaveBase... REVISION DETAIL https://phabricator.kde.org/D6832 To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6831: Make use of kauth helper in methods of file ioslave
dfaure added a comment. If you agree that showing an error message after hitting Cancel on the kauth prompt is suboptimal (I didn't actually test it), then it seems to me that a simple solution to turn `bool isPrivilegeOperationAllowed()` into a method that returns an enum? (same for `execWithElevatedPrivilege`) The values would be - Allowed (-> proceed) - Not allowed / not supported ( -> show error) - Canceled (-> don't show error) It's already a string true/false in the socket, that's easily extendable. REVISION DETAIL https://phabricator.kde.org/D6831 To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6833: Add support for PrivilegeExecution in KIO jobs
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Looks OK to me now (just minor things)
INLINE COMMENTS
> copyjob.cpp:267
>
> +KIO::JobFlag privilegeExecFlag() { return m_privilegeExecutionEnabled ?
> PrivilegeExecution : DefaultFlags; }
> +
mark the method as const
> mkpathjob.cpp:131
> Q_D(MkpathJob);
> +
> if (job->error() && job->error() != KIO::ERR_DIR_ALREADY_EXIST) {
revert unnecessary newline changes to this file
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D6833
To: chinmoyr, dfaure, #frameworks
Cc: #frameworks
D6830: Make use of kauth helper in copy method of file ioslave
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. You wrote: "For the case you mentioned kauth prompt will always be triggered during opening of file for writing." I disagree. My example case is a FAT permission where the user has write permissions, but chown/chgrp/chmod fails due to FAT not supporting these operations. Please test it. If you don't have a FAT partition, here's how to create one inside a file. dd if=/dev/zero of=/tmp/fatdevice bs=4k count=100 mkfs.vfat /tmp/fatdevice mkdir /tmp/fat sudo mount -o loop,uid=$UID /tmp/fatdevice /tmp/fat ls -lad /tmp/fat kioclient5 copy ~/.bashrc /tmp/fat/destfile When I do this, I get this warning from kio_file (in ~/.xsession-errors), which is, thankfully, ignored (no user message box). FileProtocol::copy: "Couldn't preserve group for '/tmp/fat/destfile'" So I would say: when writing worked without privilege elevation, we should keep the current logic where chown/chgrp/chmod/utime errors are ignored. On the other hand, if writing required kauth, then we can keep using kauth for those other operations (they won't prompt). REVISION DETAIL https://phabricator.kde.org/D6830 To: chinmoyr, dfaure, #frameworks Cc: elvisangelaccio, #frameworks
D6829: Add ability to use the new kauth helper in file ioslave
dfaure accepted this revision. BRANCH master REVISION DETAIL https://phabricator.kde.org/D6829 To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6833: Add support for PrivilegeExecution in KIO jobs
chinmoyr updated this revision to Diff 18019. chinmoyr added a comment. - Add PrivilegeExecution flag to subjobs - Removed the default case from CopyJob - Initially I didn't add checks for PrivilegeExecution flag in SimpleJob as most of them don't take it as argument. Now added checks for this flag for symlink, rename and delete operation. - Pass 'flags' as argument when creating a SimpleJob for rename operation. Otherwise rename would fail when inside root owned location. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6833?vs=17225=18019 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6833 AFFECTED FILES src/core/chmodjob.cpp src/core/copyjob.cpp src/core/deletejob.cpp src/core/filecopyjob.cpp src/core/job_p.h src/core/mkpathjob.cpp src/core/simplejob.cpp src/core/storedtransferjob.cpp To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D7253: Add build-flatpak target if there is a flatpak recipe around
apol added a comment. In https://phabricator.kde.org/D7253#134653, @elvisangelaccio wrote: > Main reason is having a simple way to run flatpak-builder with the proper arguments. Otherwise you have to copy-paste the command from somewhere (which could be error-prone). > One usually already has a build folder around, so the idea is you change some code, build it as usual and then you run this target to quickly test the change in flatpak. Note that this is forcing that the file is in the root directory, this doesn't need to be necessarily the case. In fact xdg-portal-kde-test has it in a subdir. This would already do what you're after: `alias fb=flatpak-builder --force-clean --ccache --require-changes --repo=repo app $@` Such an alias/script could go in here: https://cgit.kde.org/kde-dev-scripts.git/ The reason why I'm discussing this is that I think one of the advantages of Flatpak REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D7253 To: elvisangelaccio, apol, #frameworks Cc: #frameworks, #build_system
D6832: Integrate new file ioslave in KIO job
chinmoyr updated this revision to Diff 18018. chinmoyr added a comment. - Fixed the issues pointed out. - Check for UnitTesting key in metadata while showing the confirmation dialog. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6832?vs=17222=18018 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6832 AFFECTED FILES src/core/job.cpp src/core/job_base.h src/core/job_p.h src/core/simplejob.cpp src/core/simplejob.h To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6831: Make use of kauth helper in methods of file ioslave
chinmoyr added a comment. > what happens if the user cancels the root-password prompt? He will then get another msg box with "cannot create directory?" even though he/she purposefully canceled the operation? When that happens the slave should use error(KIO::ERR_USER_CANCELED) instead (which means no error box), but of course only in that case... With current version of my code this is not possible. The method `isPrivilegeOperationAllowed` returns false when user hits cancel or when PrivilegeExecution flag isn't set. To distinguish between them some kind of data needs to be sent to slave. Here `addMetaData` doesn't work. So another signal is needed. Or maybe `readData` can help here. REVISION DETAIL https://phabricator.kde.org/D6831 To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6831: Make use of kauth helper in methods of file ioslave
chinmoyr updated this revision to Diff 18016. chinmoyr added a comment. - Fixed error handling logic in FileProtocol::chmod. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6831?vs=17019=18016 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6831 AFFECTED FILES src/ioslaves/file/file.cpp src/ioslaves/file/file_unix.cpp To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6830: Make use of kauth helper in copy method of file ioslave
chinmoyr updated this revision to Diff 18015. chinmoyr added a comment. Minor changes CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6830?vs=17782=18015 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6830 AFFECTED FILES src/ioslaves/file/file_unix.cpp To: chinmoyr, dfaure, #frameworks Cc: elvisangelaccio, #frameworks
D6830: Make use of kauth helper in copy method of file ioslave
chinmoyr added a comment. For the case you mentioned kauth prompt will always be triggered during opening of file for writing. Operations like chmod, chown, utime will work (and then fail) without showing any prompt. REVISION DETAIL https://phabricator.kde.org/D6830 To: chinmoyr, dfaure, #frameworks Cc: elvisangelaccio, #frameworks
D6829: Add ability to use the new kauth helper in file ioslave
chinmoyr updated this revision to Diff 18014. chinmoyr added a comment. - added @since 5.38 - rearranged code-blocks in execWithElevatedPrivilege() method. Now the prompt is also tested in unit test mode.\ https://phabricator.kde.org/D6832 will add relevant code for testing confirmation. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6829?vs=17781=18014 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6829 AFFECTED FILES src/core/slavebase.cpp src/core/slavebase.h src/core/slaveinterface.cpp src/core/slaveinterface.h src/ioslaves/file/file.h src/ioslaves/file/file_unix.cpp src/ioslaves/file/file_win.cpp To: chinmoyr, dfaure, #frameworks Cc: #frameworks
D6197: Add kauth helper to file ioslave
chinmoyr updated this revision to Diff 18012. chinmoyr added a comment. Improved error checking in helper. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6197?vs=17780=18012 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6197 AFFECTED FILES src/ioslaves/file/CMakeLists.txt src/ioslaves/file/file_p.h src/ioslaves/file/kauth/CMakeLists.txt src/ioslaves/file/kauth/file.actions src/ioslaves/file/kauth/filehelper.cpp src/ioslaves/file/kauth/filehelper.h To: chinmoyr, elvisangelaccio, #frameworks, dfaure
D7253: Add build-flatpak target if there is a flatpak recipe around
elvisangelaccio added a comment. Main reason is having a simple way to run flatpak-builder with the proper arguments. Otherwise you have to copy-paste the command from somewhere (which could be error-prone). One usually already has a build folder around, so the idea is you change some code, build it as usual and then you run this target to quickly test the change in flatpak. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D7253 To: elvisangelaccio, apol, #frameworks Cc: #frameworks, #build_system
D7253: Add build-flatpak target if there is a flatpak recipe around
apol added a comment. Interesting feature. I'm not opposed to it, but I have the feeling that it belongs to a different layer, so this means creating a build directory to call flatpak to call cmake. :P Also the arguments one passes to cmake won't have any effect on the flatpak. What's the reason you want to work on it? maybe we should work in simplifying the flatpak-builder call? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D7253 To: elvisangelaccio, apol, #frameworks Cc: #frameworks, #build_system
D7253: Add build-flatpak target if there is a flatpak recipe around
elvisangelaccio added reviewers: apol, Frameworks. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D7253 To: elvisangelaccio, apol, #frameworks Cc: #frameworks, #build_system
D7253: Add build-flatpak target if there is a flatpak recipe around
elvisangelaccio created this revision. Restricted Application added projects: Frameworks, Build System. Restricted Application added subscribers: Build System, Frameworks. REVISION SUMMARY If there is a `org.kde.*.json` file in the git repository of the cmake project, we assume is a flatpak recipe and we add an optional target that runs `flatpak-builder` on it. This way we don't have to copy `build.sh` scripts all over the place. TEST PLAN 1. git clone the ark repo 2. run cmake 3. make/ninja build-flatpak REPOSITORY R240 Extra CMake Modules BRANCH master REVISION DETAIL https://phabricator.kde.org/D7253 AFFECTED FILES kde-modules/KDECMakeSettings.cmake To: elvisangelaccio Cc: #frameworks, #build_system
KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 44 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/44/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Fri, 11 Aug 2017 16:05:56 + Build duration: 1 min 18 sec and counting JUnit Tests Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s) Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report100% (1/1)80% (4/5)80% (4/5)57% (163/287)34% (79/234)Coverage Breakdown by Package Name FilesClassesLinesConditionalssrc80% (4/5)80% (4/5)57% (163/287)34% (79/234) build.log Description: Binary data
KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 40 - Still Unstable!
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/40/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Fri, 11 Aug 2017 16:05:56 + Build duration: 54 sec and counting JUnit Tests Name: (root) Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: TestSuite.qmltests build.log Description: Binary data
D7127: ignore spurious resize events to empty sizes
cfeck added a comment. The commit still says "size != oldSize". Is this correct? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7127 To: mart, #plasma, davidedmundson Cc: cfeck, davidedmundson, broulik, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7250: Avoid some unnecessary theme content lookups
fvogt closed this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7250 To: fvogt, #plasma, mart Cc: #frameworks
D7250: Avoid some unnecessary theme content lookups
mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D7250 To: fvogt, #plasma, mart Cc: #frameworks
D7250: Avoid some unnecessary theme content lookups
fvogt added a reviewer: Plasma. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7250 To: fvogt, #plasma Cc: #frameworks
D7250: Avoid some unnecessary theme content lookups
fvogt updated this revision to Diff 18004. fvogt added a comment. Forgot a check REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7250?vs=18003=18004 BRANCH master REVISION DETAIL https://phabricator.kde.org/D7250 AFFECTED FILES src/plasma/svg.cpp To: fvogt Cc: #frameworks
D7250: Avoid some unnecessary theme content lookups
fvogt created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY imagePath can be an absolute path into an iconTheme -> Do not try to find it in the Plasma theme imagePath can be empty -> Do not try to look it up at all REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D7250 AFFECTED FILES src/plasma/svg.cpp To: fvogt Cc: #frameworks
D7249: Return high-resolution line edit clear icon
broulik created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Qt just returns a 16px pixmap by default leading to blurry results when larger icon sizes for small icons are configured by the user. TEST PLAN Originally posted as https://git.reviewboard.kde.org/r/128895 I'm still seeing the issue without this patch. @hpereiradacosta In Dolphin press Ctrl+F to enter the search, the `KUrlComboBox` in the address bar does *not* exhibit this bug as `KLineEdit` does not use `QLineEdit`'s action feature but its own `KLineEditButton` REPOSITORY R252 Framework Integration REVISION DETAIL https://phabricator.kde.org/D7249 AFFECTED FILES src/kstyle/kstyle.cpp To: broulik, kde-frameworks-devel, hpereiradacosta Cc: hpereiradacosta, #frameworks
D7237: KIO/Mac : make kiod5 an "agent"
rjvbb added a comment. In https://phabricator.kde.org/D7237#134438, @dfaure wrote: > Forgot to git add kiod_agent.mm? In fact, I see I have a comparable patch for kioslave, but simpler because AFAICT it won't ever put up a GUI itself. What would be an appropriate place in KIO/core where we could put the 2 functions so they can be shared? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7237 To: rjvbb, #frameworks, dfaure Cc: kde-mac, dfaure, #frameworks
D7237: KIO/Mac : make kiod5 an "agent"
rjvbb updated this revision to Diff 17997. rjvbb edited the summary of this revision. rjvbb added a comment. Forgot a `git add` indeed, as well as a final bit of proofreading. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7237?vs=17981=17997 REVISION DETAIL https://phabricator.kde.org/D7237 AFFECTED FILES src/kiod/CMakeLists.txt src/kiod/kiod_agent.mm src/kiod/kiod_main.cpp To: rjvbb, #frameworks, dfaure Cc: kde-mac, dfaure, #frameworks
D7237: KIO/Mac : make kiod5 an "agent"
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7237 To: rjvbb, #frameworks, dfaure Cc: kde-mac, dfaure, #frameworks
D7245: Improve reStructuredText highlighting
dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. In general ok, but there are two issues to be fixed 1. Remove spaces around items as noted in the comment 2. Please extend / add a highlighting test case in autotest/input/ The goal is to have all highlighting files unit tested, since otherwise we can not maintain this over time. INLINE COMMENTS > rest.xml:21 > + > + attention > + caution Please remove the space before and after attention, i.e. " attention " -> "attention". Syntax highlighting files with spaces raise warnings and soon will not be accepted anymore. The reason for this change was that when loading the xml files, we want to avoid to trim hundreds of thousands of QStrings. Same for all the office items. REVISION DETAIL https://phabricator.kde.org/D7245 To: turbov, #kate, #framework_syntax_hightlighting, dhaumann Cc: dhaumann, #frameworks
D6233: KKeyServer: fix handling of KeypadModifier.
This revision was automatically updated to reflect the committed changes. Closed by commit R278:32526718eae9: KKeyServer: fix handling of KeypadModifier. (authored by dfaure). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6233?vs=15522=17994#toc REPOSITORY R278 KWindowSystem CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6233?vs=15522=17994 REVISION DETAIL https://phabricator.kde.org/D6233 AFFECTED FILES autotests/CMakeLists.txt autotests/kkeyserver_x11_unittest.cpp src/platforms/xcb/kkeyserver.cpp src/platforms/xcb/kkeyserver_x11.h To: dfaure, graesslin Cc: graesslin, #frameworks
