D21661: add snoretoast backend for KNotifications on Windows
brute4s99 updated this revision to Diff 60035. brute4s99 marked 9 inline comments as done. REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21661?vs=59835=60035 BRANCH arcpatch-D21661 REVISION DETAIL https://phabricator.kde.org/D21661 AFFECTED FILES src/CMakeLists.txt src/knotification.cpp src/knotificationmanager.cpp src/notifybysnore.cpp src/notifybysnore.h To: brute4s99, broulik, sredman, vonreth, albertvaka Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D21661: add snoretoast backend for KNotifications on Windows
brute4s99 added inline comments. INLINE COMMENTS > pino wrote in notifybysnore.cpp:84 > if the notification is not found, this will be an uninitialized pointer; TBH > if the search for the notification with the specified id fails, then it > should be better to return earlier, as it means the notification is unknown well, I just found out the patch had broken functionality that I fixed just after putting here an `else return`! > pino wrote in notifybysnore.cpp:168-171 > the logic here is swapped: if `waitForStarted()` returns false, that means > the process did not start successfully; also, after `finish()` you must > return earlier (do not forget to delete the process), otherwise the rest of > the code does things as if the process run fine I'm removing this code chunk because we already check for successful show of notif in the following `connect`. > pino wrote in notifybysnore.cpp:44 > > if you could guide me on how to update the docs on the website, that'd be > > great! > > which website? https://api.kde.org/frameworks/knotifications/html/index.html REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21661 To: brute4s99, broulik, sredman, vonreth, albertvaka Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D17595: Upstream Dolphin's file rename dialog
ngraham added a comment. Who's left? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham
D21607: Don't delay emission of matchesChanged indefinitely
fvogt added a comment. Before I land this, I'd like if someone other than me tries krunner with this patch applied and judges the result with several runners. The difference is very noticable with the appstream runner as it does not batch results. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D21607 To: fvogt, #frameworks, broulik Cc: ngraham, bruns, kde-frameworks-devel, LeGast00n, michaelh
D14467: Auth Support: Drop privileges if target is not owned by root
maltek requested changes to this revision.
maltek added a comment.
This revision now requires changes to proceed.
I've gone over the code and found some issues. I haven't fully thought
through the design on a conceptual level, because I assume Matthias already did.
INLINE COMMENTS
> filehelper.cpp:74
> +} else {
> +target_fd = openat(parent_fd, baseName.data(), O_NOFOLLOW);
> +}
This is still subject to race conditions. You're opening the file with
O_NOFOLLOW and calling fstat on it, correctly. But then, you don't return this
file descriptor to further work on it. So there's no way to know if the file
checked here and the file that's changed later on are the same.
(It's also leaking the opened file descriptor, currently.)
> filehelper.cpp:105
> +if (setegid(newgid) == -1 || seteuid(newuid) == -1) {
> +return false;
> +}
After this `return false`, the process is in some undefined state with unknown
groups, since there is no attempt to restore the original groups. The chance of
these calls failing are quite slim, however - it can only really happen due to
programming error when the program doesn't have privileges to call sete[ug]id.
Personally, I'd just abort the program in such circumstances, but since it
should never be happening, reasonable people might disagree.
> filehelper.cpp:144
> +int mode = arg2.toInt();
> +if (fchmodat(parent_fd, baseName.data(), mode,
> AT_SYMLINK_NOFOLLOW) == -1) {
> +reply.setError(errno);
AT_SYMLINK_NOFOLLOW is not implemented for fchmodat, so this will always fail
with ENOTSUP. It seems the only safe way to do this is to open() the file with
O_NOFOLLOW, and then use fchmod().
> filehelper.cpp:222
> +times[1].tv_sec = modtime * 1000;
> +times[1].tv_nsec = modtime / 1000;
> +if (utimensat(parent_fd, baseName.data(), times,
> AT_SYMLINK_NOFOLLOW) == -1) {
I *think* the divisions/multiplications are reversed here.
> filehelper.cpp:231
> }
> -};
>
> +close(parent_fd);
Somewhere here, I'd expect the privileges to be escalated again, to restore the
state from before the call to dropPrivileges().
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D14467
To: chinmoyr, dfaure, ngraham, elvisangelaccio, #frameworks, #dolphin, maltek
Cc: maltek, mreeves, mgerstner, fvogt, kde-frameworks-devel, LeGast00n,
michaelh, ngraham, bruns
D21721: [WIP] Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)
pino added inline comments.
INLINE COMMENTS
> engine.h:471
> + */
> +virtual CommentsModel* commentsForEntry(const KNSCore::EntryInternal
> );
> +
BIC change, you cannot add virtual functions in a public class
> provider.h:148
> virtual void loadPayloadLink(const EntryInternal , int linkId) = 0;
> +virtual void loadComments(const EntryInternal &, int, int) {};
>
BIC change, you cannot add virtual functions in a public class
REPOSITORY
R304 KNewStuff
REVISION DETAIL
https://phabricator.kde.org/D21721
To: leinir
Cc: pino, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D21882: RFC: [CopyJob] Batch reporting processed amount
broulik created this revision. broulik added reviewers: Frameworks, dfaure, chinmoyr. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY While reporting the number of files is batched, the processed amount of bytes is relayed immediately, causing excessive DBus traffic when copying many small files. TEST PLAN Copied some stuff from PC to laptop and noticed massive dbus traffic on the jobviewinterface. Tests still pass REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21882 AFFECTED FILES src/core/copyjob.cpp To: broulik, #frameworks, dfaure, chinmoyr Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D21721: [WIP] Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)
leinir updated this revision to Diff 60021. leinir added a comment. Highlights: NewStuffButton, basic Kiosk support (needs more clever and user-facing mention of why the thing they just tried to do didn't happen/do anything), various fixing, cleanup, and sanity work. - Fix logic for canFetchMore (initial state was a touch funny) - Explicitly request the listed categories for searching - Fix the emptiness issue in the proper place, and properly - Don't need to set this, now it's a proper model - Slightly more roundabout, but saner way for the API to handle the engine - Missed two files in previous commit - Allow opening any random knsrc file on the user's system - Expose a changedEntries property on the KNSQuick Engine - Import the proper NewStuff version - Add the initial version of a KNSQuick Button - Add a NewStuff.Button to the khns-dialog test app - Add allowedByKiosk in the Quick engine, and expose it - Add allowedByKiosk, and mark a few things invokable - Add a NewStuffDialog component (and content) - Minor doc change - Add the NewStuffDialog and NewStuffDialogContent components - Much simpler NewStuffButton (and use the Engine's kiosk info) REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21721?vs=59510=60021 BRANCH knsquick-feature-parity-with-kns (branched from master) REVISION DETAIL https://phabricator.kde.org/D21721 AFFECTED FILES src/attica/atticaprovider.cpp src/attica/atticaprovider_p.h src/core/CMakeLists.txt src/core/commentsmodel.cpp src/core/commentsmodel.h src/core/engine.cpp src/core/engine.h src/core/entryinternal.cpp src/core/installation.cpp src/core/itemsmodel.cpp src/core/itemsmodel.h src/core/provider.h src/core/question.h src/qtquick/CMakeLists.txt src/qtquick/categoriesmodel.cpp src/qtquick/categoriesmodel.h src/qtquick/qml/ConditionalLoader.qml src/qtquick/qml/EntryCommentDelegate.qml src/qtquick/qml/EntryCommentsPage.qml src/qtquick/qml/EntryScreenshots.qml src/qtquick/qml/GridTileDelegate.qml src/qtquick/qml/NewStuffButton.qml src/qtquick/qml/NewStuffDialog.qml src/qtquick/qml/NewStuffDialogContent.qml src/qtquick/qml/NewStuffDownloadItemsSheet.qml src/qtquick/qml/NewStuffEntryDetails.qml src/qtquick/qml/NewStuffPage.qml src/qtquick/qml/NewStuffQuestionAsker.qml src/qtquick/qml/Rating.qml src/qtquick/qml/Shadow.qml src/qtquick/qmldir src/qtquick/qmlplugin.cpp src/qtquick/quickengine.cpp src/qtquick/quickengine.h src/qtquick/quickitemsmodel.cpp src/qtquick/quickitemsmodel.h src/qtquick/quickquestionlistener.cpp src/qtquick/quickquestionlistener.h tests/CMakeLists.txt tests/khotnewstuff-dialog-ui/main.qml tests/khotnewstuff-dialog.cpp To: leinir Cc: ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D17595: Upstream Dolphin's file rename dialog
meven planned changes to this revision. meven added a comment. Still need 4 agreements from copyrights holders to make the necessary license change. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17595 To: meven, #frameworks, #dolphin, broulik, ngraham Cc: emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham
D21660: change audio dep logic wrt win32
bcooksley added a comment. With regards to Windows, please note that any unit test which depends on calls that involve D-Bus on the CI system will likely lead to that test hanging because dbus-daemon is not launched by the CI system. Where possible D-Bus should be avoided on Windows. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
KDE CI: Frameworks » kservice » kf5-qt5 FreeBSDQt5.12 - Build # 31 - Still Unstable!
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kservice/job/kf5-qt5%20FreeBSDQt5.12/31/ Project: kf5-qt5 FreeBSDQt5.12 Date of build: Tue, 18 Jun 2019 10:49:21 + Build duration: 2 min 30 sec and counting JUnit Tests Name: projectroot Failed: 2 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 9 test(s)Failed: projectroot.autotests.kmimeassociationstestFailed: projectroot.autotests.ksycoca_xdgdirstestName: projectroot.tests Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)
D21780: Add X-Flatpak-RenamedFrom as recognized key
This revision was automatically updated to reflect the committed changes. Closed by commit R309:d39cabf2ae86: Add X-Flatpak-RenamedFrom as recognized key (authored by broulik). REPOSITORY R309 KService CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21780?vs=59727=60015 REVISION DETAIL https://phabricator.kde.org/D21780 AFFECTED FILES src/services/application.desktop To: broulik, #frameworks, dfaure, sitter Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D17241: WIP:Disable highlighting after 512 characters on a line.
sars added a comment. I think a partially highlighted line is better than a totally non-highlighted one. And I think that the user is more likely to instinctively guess correctly why the end of the line is not highlighted than if the line is not highlighted at all. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
Re: KInit - Current state and benchmarks
> Are we sure it's fair to assume people have SSD? our of the 4 laptops i own, > only 2 have SSD. It's at least safe to assume it's the trend moving forward. > Do you think it's worth me trying in one of the two that don't have SSD? More data is normally a good thing. If you or anyone else wants to collect stats: >From my git link above, it's as simple as running the normal ; cmake; make ; ./kinittest -median 5
D21660: change audio dep logic wrt win32
brute4s99 marked an inline comment as done. brute4s99 added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:42 > We don't need DBus on Windows, do we? we don't, I guess, but dbus-daemon.exe still runs in the background so I can't say. The functionality doesn't seem to have been affected by removing this in my build, though. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik Cc: apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
Re: KInit - Current state and benchmarks
On Monday, 17 June 2019 21:34:38 CEST David Edmundson wrote: > > Which libraries are covered by this mechanism nowadays? The impact is of > > course bigger the more of the dependencies of the applications are already > > loaded. When this was developed this was a small amount of relatively > > large Qt and kdelibs libraries. I'm wondering if the current subset is > > still relevant, both from Qt (e.g. thinking about QML/Qt Quick) and KF5? > > From what I can tell: > > implicitly linked to kdeinit: > QtBase > QtGui > Crash > I18n > ConfigCore > WindowSystem > > explicitly added at runtime: > KIOCore > Parts > Plasma > > and all the dependencies thereof. > Note libplasma doesn't link QML/Qtquick ah, I missed the runtime part when searching for this. So that covers a lot of KF5 (via the extensive dependencies of Parts) I think, as well as QtWidgets. Newer Kirigami-based applications that are more interesting for Plasma Mobile would likely benefit much less from this compared to XMLGUI desktop applications (which are less relevant on resource-constraint mobile/embedded platforms). And Plasma itself is also not be optimally served by not covering QtQuick I guess. Seems like this would need some refocusing if we want to keep it? Regards, Volker signature.asc Description: This is a digitally signed message part.
