closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread René J . V . Bertin
Morning!

Is there anything like a closest equivalent to KApplicationPrivate::init() in 
KF5 Frameworks?
I added a "bring application to the foreground" call (to a KWindowSystem helper 
function) in there, which alleviated the issue of applications remaining 
somewhere in the background on OS X. 
I'm still keeping an eye out for central locations where I could call that 
helper function.

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126403: Get rid of QApplication dependency

2016-01-02 Thread Thomas Lübking

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126403/
---

(Updated Jan. 2, 2016, 9:57 a.m.)


Review request for KDE Frameworks, kwin and Albert Astals Cid.


Bugs: 354811
https://bugs.kde.org/show_bug.cgi?id=354811


Repository: kwindowsystem


Description
---

summarized, alternative to https://git.reviewboard.kde.org/r/126397/

NOTICE: this compiles but is otherwise *completely* untested!


Diffs (updated)
-

  src/platforms/xcb/kwindowsystem.cpp 9d28704 

Diff: https://git.reviewboard.kde.org/r/126403/diff/


Testing
---

no, not the least. esp. not on the bug.


Thanks,

Thomas Lübking

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126595: [KFileMetaData] Allow querying for a file's origin URL

2016-01-02 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126595/#review90435
---



src/properties.h (line 215)


adding an enum value in the middle breaks API


- David Edmundson


On Jan. 2, 2016, 1:32 a.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126595/
> ---
> 
> (Updated Jan. 2, 2016, 1:32 a.m.)
> 
> 
> Review request for KDE Frameworks and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This attribute is set, for example by Chrome and newer versions of wget, and 
> indicates the original URL this file was downloaded from.
> 
> https://wiki.freedesktop.org/www/CommonExtendedAttributes/
> 
> 
> Diffs
> -
> 
>   src/properties.h 64ba0be 
>   src/propertyinfo.cpp 0b298b6 
>   src/usermetadata.h ab58e16 
>   src/usermetadata.cpp 51c87f8 
> 
> Diff: https://git.reviewboard.kde.org/r/126595/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126426: Add a warning color to kwalletd's password dialogs

2016-01-02 Thread Elvis Angelaccio


> On Jan. 1, 2016, 5:04 p.m., David Faure wrote:
> > Ship It!

David, could you have a look to RR 126505, before I ship this one?
(btw, the 5.18 tagging is today?)


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126426/#review90400
---


On Dec. 24, 2015, 5:55 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126426/
> ---
> 
> (Updated Dec. 24, 2015, 5:55 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> Set a "background warning color" when password and its verification do not 
> match, in the KNewPasswordDialog run by kwalletd.
> This adds a new dependency (needed to avoid hardcoding colors), but I think 
> is a nice feature to have.
> See RR 125619 for a screenshot: 
> https://git.reviewboard.kde.org/r/125619/file/2515/
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/CMakeLists.txt 
> ba9e7ba508c74fed1d8101496583061245640aa7 
>   src/runtime/kwalletd/kwalletd.cpp 6da97031e13240a9630cfa7e0dc3cf42575819c4 
> 
> Diff: https://git.reviewboard.kde.org/r/126426/diff/
> 
> 
> Testing
> ---
> 
> Compiles fine.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4support

2016-01-02 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126507/#review90439
---



src/kdeui/k4style.cpp (line 1176)


The double-lookup looks slow and unnecessary.
contains + *object doesn't give you anything that object (followed by a 
dereference if not null) gives you.

If the fear is concurrent access, then contains + *object can still 
dereference a null pointer anyway.

But K4Style runs in the GUI thread always anyway. So I would call object(), 
put that into a pointer, and then if not null, make a copy into a value. This 
avoids the double lookup.


- David Faure


On Jan. 2, 2016, 1:55 a.m., Michael Pyne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126507/
> ---
> 
> (Updated Jan. 2, 2016, 1:55 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> Fix a couple of Coverity issues:
> 
> 1. CID 1175508; file descriptors used in KLockFile could be leaked in
> error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl
> also checks that the lock has been taken, which is only considered true
> if LockOK had been returned in our lock function. Instead close() the fd
> ourselves unless we make it to LockOK.
> 
> 2. CID 117; The standard mis-use of QCache. QCache::insert can, in
> theory, delete our object as soon as we insert it into cache, so we have
> to check for that. Even ::contains() and ::object() can be risky (the
> pointers returned by object() have no lifetime guarantee), but since
> this is GUI code I assume it's only used single-threaded and not
> re-entrant. Otherwise we'd need even more paranoia...
> 
> 
> Diffs
> -
> 
>   src/kdecore/klockfile_unix.cpp 67283a5 
>   src/kdeui/k4style.cpp a1a2ab1 
> 
> Diff: https://git.reviewboard.kde.org/r/126507/diff/
> 
> 
> Testing
> ---
> 
> Everything builds and appears to still work, though it's hard to test K4Style 
> as I'm not sure what uses it right at this point.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126505: Do not show a warning color before the user even started typing

2016-01-02 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126505/#review90441
---

Ship it!


Ship It!

- David Faure


On Dec. 27, 2015, 2:40 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126505/
> ---
> 
> (Updated Dec. 27, 2015, 2:40 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability, Christoph Feck, and David 
> Faure.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> As discussed in RR 125619 and 126426, the password verification field (in a 
> KNewPasswordWidget) should not be marked as "wrong" before the user even 
> started typing the verification password.
> 
> The revised approach is the following:
> 
> * The user starts typing something as password, e.g. 1234
> * The user types something else as verification password
> * As soon as the verification is not anymore a prefix of the password (e.g. 
> verification = 122), the warning color is shown.
> * As soon as the verification is a prefix again (e.g. the user deletes the 
> second 2, i.e. verification = 12) the warning color is not shown anymore.
> 
> 
> Diffs
> -
> 
>   autotests/knewpasswordwidgettest.h 43845128adec01aced4353c9f7986b7977829a2a 
>   autotests/knewpasswordwidgettest.cpp 
> 297b88d5f18b9cd37f0d26d94e56f38870756f20 
>   src/knewpasswordwidget.cpp a1b59454a2c2d7c09ac32acec52d3fffa73f77fc 
> 
> Diff: https://git.reviewboard.kde.org/r/126505/diff/
> 
> 
> Testing
> ---
> 
> Autotests assert what described above. Gif pictures would explain the patch 
> better than 1000 words, but I suck at creating them :(
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126161: OS X housekeeping

2016-01-02 Thread David Faure


> On Dec. 25, 2015, 2:42 p.m., René J.V. Bertin wrote:
> > src/kdeinit/kinit_mac.mm, lines 662-666
> > 
> >
> > I'd love to add `[NSApp activateIgnoringOtherApps:YES]` and/or `[NSApp 
> > unhide]` here, preceded by `[NSApplication sharedApplication]` so that the 
> > spawned `executable` appears in front and not behind the windows of the 
> > "parent" application.
> > 
> > I think that's a very sensible thing to do, but sadly those calls are 
> > part of (or call into) the SDKs that are off-limits between a `fork()` and 
> > an `exec()`.
> > 
> > This really makes me reconsider to use a wrapper, because it quickly 
> > grows old 1) having to wait for an expected application to appear, 2) 
> > realise it must have opened somewhere in the background and 3) go dig it up.
> > 
> > If only I could be sure that the spawned application is NOT supposed to 
> > inherit the environment and other context from kdeinit5. In that case I 
> > could rewrite the command to execute so that it uses LaunchServices ... via 
> > a proxy that's already available (`/usr/bin/open`).
> > 
> > NB: this may well be why `[NSProcessInfo setProcessName:]` doesn't 
> > appear to have the intended effect.
> > 
> > So, David, what's worse here — introducing an additional layer or 
> > putting up with applications that play hide and seek?

Starting apps via kdeinit is only an optimization. It's perfectly OK to start 
apps directly with QProcess instead. So yes, the spawned application is NOT 
supposed to inherit the environment from kdeinit5.

As previously stated, I strongly recommend to avoid the whole complication of 
"stuff I can't do between fork and exec" on OSX, but not using fork and exec at 
all (wrapper or not), and just starting the other process directly with 
QProcess. If that doesn't bring the new app to the front, maybe that's a 
QProcess bug or missing feature? If you think about this as a "pure Qt 
application developer", such a developer would expect QProcess to be able to 
start a GUI app and have it pop up to the front. Maybe you can write a small 
test with QProcess, to check if that works out of the box (maybe all your 
troubles actually come from the indirection via kdeinit...)


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126161/#review90098
---


On Nov. 26, 2015, 4:20 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126161/
> ---
> 
> (Updated Nov. 26, 2015, 4:20 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> This patch addresses several issues with the OS X adaptations:
> 
> -1 replaces the obsolete Q_OS_MAC with Q_OS_OSX
> -2 builds the relevant applications `nongui` instead of as app bundles
> -3 turns klauncher into an "agent" by setting `LSUIElement` to true 
> programmatically
> -4 ports a patch that has been in MacPorts' `port:kdelibs4` since October 
> 14th 2009, which prevents a kdeinit crash that is caused by calling exec 
> after `fork()` in an application that has used non-POSIX APIs and/or calling 
> those APIs in the exec'ed application. This patch (originally by MacPorts 
> developers Jeremy Lainé and Jeremy Lavergne) rearranges call order and uses a 
> proxy application to do the actual exec.
> 
> 
> Diffs
> -
> 
>   src/kdeinit/CMakeLists.txt f94db71 
>   src/kdeinit/kdeinit5_proxy.mm PRE-CREATION 
>   src/kdeinit/kinit.cpp a18008a 
>   src/kdeinit/kinit_mac.mm PRE-CREATION 
>   src/klauncher/CMakeLists.txt 746edfa 
>   src/klauncher/klauncher.h e155f72 
>   src/klauncher/klauncher.cpp 8b3d343 
>   src/klauncher/klauncher_main.cpp f69aaa5 
>   src/start_kdeinit/CMakeLists.txt 46d6cb3 
>   src/wrapper.cpp 95b7ec2 
> 
> Diff: https://git.reviewboard.kde.org/r/126161/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Qt 5.5.1 and KF5rameworks 5.16.0 . With this patch, 
> starting `kded5` will launch kdeinit5 and klauncher5 as expected, but 
> `kdeinit5 --kded` does not yet launch `kded5`. This is probably acceptable 
> for typical KF5 use on OS X (kded5 can be launched as a login item or as a 
> LaunchAgent) but I will have another look at why the kded isn't started.
> 
> I am not yet able to perform further testing; practice will for instance have 
> to show whether point 2 above needs revision (apps that need to be installed 
> as app bundles).
> 
> Similarly it will have to be seen whether point 3 above has any drawbacks. 
> Applications running as agents do not show up in the Dock or App Switcher. 
> Thus

Re: Review Request 126170: [OS X] make kded5 an agent, and build it as a regular application instead of an app bundle

2016-01-02 Thread David Faure


> On Dec. 2, 2015, 7:51 a.m., David Faure wrote:
> > Please kind in mind that kded must be able to pop up dialogs, though.
> > (cookie dialog, SSL cert messagebox + dialog, etc. etc.).
> > 
> > If making it an "agent" doesn't prevent it from showing GUI elements now 
> > and then, then no problem.
> 
> René J.V. Bertin wrote:
> With the earlier approach of setting `LSUIElement` that is guaranteed to 
> be the case.
> 
> I just checked; launching Qt's Assistant with 
> `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM=1` all that changes is that 
> the application remains in the background; it can be brought into the 
> foreground, and it retains its presence in the Dock and app switcher.
> 
> IOW, I'm not really sure I understand why kded5 doesn't retain that 
> presence with `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM` set. It's 
> possible that all the env. variable does is postpone the actions that lead to 
> that presence. If that's true than we'd have to come back to the more 
> appropriate previous revision of this patch.
> 
> OTOH: the only dialogs I have seen under KDE4 that are related to kded 
> (unknown cert) were posted when kded4 was *not* running. Ditto for cookie 
> related things. Under what circumstances is kded supposed to present a GUI?
> 
> David Faure wrote:
> Here is an easy way to test this: do the same change for kiod in kio 
> (it's like a mini kded) and then
> cd kio/tests ; ./listjobtest ftp://t...@upload.kde.org
> should bring up a password dialog.
> 
> Except that with Qt 5.6 from git here (from some time ago) it asserts in 
> DBus (looking into that now)... but hopefully you have Qt 5.5 ?
> 
> René J.V. Bertin wrote:
> OK, here's a reason NOT to use 
> QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM: it has to be unset, and I'm 
> not sure when this would have to/could be done such that the variable is 
> picked up for kded5 itself, but not by anything that is launched subsequently.
> 
> If kded5 is used to start kdeinit5 for instances, everything launched 
> through there will launch in the background.
> 
> So I'm going to go back to the original proposition, because an 
> LSUIElement app is exactly what kded ought to be.
> 
> David Faure wrote:
> I don't understand what you're saying. kded5 isn't starting any other 
> processes, is it?
> 
> René J.V. Bertin wrote:
> kdeinit5 is auto-started as part of what I think is normal behaviour. So 
> if kded5 is started first, that's what starts kdeinit5. Shouldn't it?
> 
> My reasoning here is that users might be interested in the possibility to 
> prepare the KDE runtime infrastructure with an inoffensive and non-invasive 
> daemon process that is part of the infrastructure itself. It is my experience 
> with KDE4/Mac that not doing so, but leaving the bootstrapping until you 
> start some larger and (much) more complex application (or suite; think 
> starting Akonadi) can lead to subtle but annoying stability issues.
> 
> NB: starting kded5 through kdeinit5 does *not* work on OS X. I've had a 
> quick look to understand why, but quickly gave up due to the complexity of 
> the code.

If a user wants to prepare the runtime infrastructure, he/she should run 
kdeinit5, not kded5.
kdeinit5 is the master of everything; kded is just a bunch of on-demand 
services.

Apps started standalone, who then make a dbus call to kded might indeed start 
kded first, which would in turn start kdeinit.
But yeah, unset any env vars in kdeinit that shouldn't be set for the apps it 
starts, that makes sense.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126170/#review89022
---


On Dec. 25, 2015, 9:14 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126170/
> ---
> 
> (Updated Dec. 25, 2015, 9:14 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kded
> 
> 
> Description
> ---
> 
> There should be no reason to build `kded5` as an app bundle on OS X, and with 
> recent feedback in mind I postulated that marking it "nongui" 
> (`ecm_mark_nongui_application`) would be acceptable on other platforms too.
> 
> Additionally, `kded5` doesn't have any more reason to appear in the Dock or 
> app switcher, on OS X, so I have added the code to make it an agent.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 5e95df8 
>   src/kded.cpp c7fdfee 
> 
> Diff: https://git.reviewboard.kde.org/r/126170/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Qt 5.5.1 and FWs 5.16.0 .
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

__

Re: Review Request 126314: Port klauncher to xcb

2016-01-02 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126314/#review90445
---

Ship it!


Ship It!

- David Faure


On Dec. 11, 2015, 3:10 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126314/
> ---
> 
> (Updated Dec. 11, 2015, 3:10 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Port klauncher to xcb
> 
> 
> Diffs
> -
> 
>   src/klauncher/klauncher.cpp 8b3d343b9fb2c852ea2d6c559f13c96a0467d72b 
>   src/klauncher/CMakeLists.txt 746edfac0b840aa033be219ae5d094689006db6f 
>   src/klauncher/klauncher.h e155f72d76d4f99172646ab797ec8c4dd006ddf7 
> 
> Diff: https://git.reviewboard.kde.org/r/126314/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread David Faure
On Saturday 02 January 2016 10:49:59 René J.V. Bertin wrote:
> Morning!
> 
> Is there anything like a closest equivalent to KApplicationPrivate::init() in 
> KF5 Frameworks?

Yes, QApplicationPrivate::init() :-)

> I added a "bring application to the foreground" call (to a KWindowSystem 
> helper function) in there, which alleviated the issue of applications 
> remaining somewhere in the background on OS X. 
> I'm still keeping an eye out for central locations where I could call that 
> helper function.

This would break focus-stealing-prevention.
Apps started while the user is typing elsewhere shouldn't be brought to the 
foreground.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126161: OS X housekeeping

2016-01-02 Thread René J . V . Bertin


> On Dec. 25, 2015, 3:42 p.m., René J.V. Bertin wrote:
> > src/kdeinit/kinit_mac.mm, lines 662-666
> > 
> >
> > I'd love to add `[NSApp activateIgnoringOtherApps:YES]` and/or `[NSApp 
> > unhide]` here, preceded by `[NSApplication sharedApplication]` so that the 
> > spawned `executable` appears in front and not behind the windows of the 
> > "parent" application.
> > 
> > I think that's a very sensible thing to do, but sadly those calls are 
> > part of (or call into) the SDKs that are off-limits between a `fork()` and 
> > an `exec()`.
> > 
> > This really makes me reconsider to use a wrapper, because it quickly 
> > grows old 1) having to wait for an expected application to appear, 2) 
> > realise it must have opened somewhere in the background and 3) go dig it up.
> > 
> > If only I could be sure that the spawned application is NOT supposed to 
> > inherit the environment and other context from kdeinit5. In that case I 
> > could rewrite the command to execute so that it uses LaunchServices ... via 
> > a proxy that's already available (`/usr/bin/open`).
> > 
> > NB: this may well be why `[NSProcessInfo setProcessName:]` doesn't 
> > appear to have the intended effect.
> > 
> > So, David, what's worse here — introducing an additional layer or 
> > putting up with applications that play hide and seek?
> 
> David Faure wrote:
> Starting apps via kdeinit is only an optimization. It's perfectly OK to 
> start apps directly with QProcess instead. So yes, the spawned application is 
> NOT supposed to inherit the environment from kdeinit5.
> 
> As previously stated, I strongly recommend to avoid the whole 
> complication of "stuff I can't do between fork and exec" on OSX, but not 
> using fork and exec at all (wrapper or not), and just starting the other 
> process directly with QProcess. If that doesn't bring the new app to the 
> front, maybe that's a QProcess bug or missing feature? If you think about 
> this as a "pure Qt application developer", such a developer would expect 
> QProcess to be able to start a GUI app and have it pop up to the front. Maybe 
> you can write a small test with QProcess, to check if that works out of the 
> box (maybe all your troubles actually come from the indirection via 
> kdeinit...)

I will indeed have to have another look at what QProcess does exactly in 
qprocess_unix, but a priori it looks a variant on the fork+exec scheme. There 
is support for app bundles, but that works by obtaining the bundle executable 
and launching it as it if were a regular application.

The underlying issue is not with Qt, it's something specific to OS X. Maybe 
there's a reason for it: it could be a cheap way to avoid stealing focus from 
the "parent" process. The app bundle detection could be used to channel 
spawning the new process through LaunchServices so that it does open in the 
foreground, and that could even be extended to cover symlinks that point inside 
an app bundle (i.e. open `foo.app` instead of 
`/path/to/foo.app/Contents/MacOS/foo_bundle_exec`). Qt's design choices may 
make that impossible though: QProcess's implementatin (notably 
`QProcessPrivate::startProcess()`) is quite complex and if it is supposed to 
support Posix-style communication between parent and child (including pipes) 
then LaunchServices is probably out of the question. I don't think there is any 
such assumption in kdeinit, correct?

I have to come back a bit on the idea of using `/usr/bin/open` : it tends to 
use Terminal.app to launch shell scripts and "certain" regular executables 
("BSD utilities" in Mac speak). That's evidently not an option. I do have 
wrapper utilities lying around that I hacked together, I'll see if and how I 
can refactor relevant code from them into kinit_mac.mm .


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126161/#review90098
---


On Nov. 26, 2015, 5:20 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126161/
> ---
> 
> (Updated Nov. 26, 2015, 5:20 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> This patch addresses several issues with the OS X adaptations:
> 
> -1 replaces the obsolete Q_OS_MAC with Q_OS_OSX
> -2 builds the relevant applications `nongui` instead of as app bundles
> -3 turns klauncher into an "agent" by setting `LSUIElement` to true 
> programmatically
> -4 ports a patch that has been in MacPorts' `port:kdelibs4` since October 
> 14th 2009, which prevents a kdein

Re: Review Request 126595: [KFileMetaData] Allow querying for a file's origin URL

2016-01-02 Thread Kai Uwe Broulik


> On Jan. 2, 2016, 9:59 vorm., David Edmundson wrote:
> > src/properties.h, line 215
> > 
> >
> > adding an enum value in the middle breaks API

where can I place it? the last entry of course must stay last.


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126595/#review90435
---


On Jan. 2, 2016, 1:32 vorm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126595/
> ---
> 
> (Updated Jan. 2, 2016, 1:32 vorm.)
> 
> 
> Review request for KDE Frameworks and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This attribute is set, for example by Chrome and newer versions of wget, and 
> indicates the original URL this file was downloaded from.
> 
> https://wiki.freedesktop.org/www/CommonExtendedAttributes/
> 
> 
> Diffs
> -
> 
>   src/properties.h 64ba0be 
>   src/propertyinfo.cpp 0b298b6 
>   src/usermetadata.h ab58e16 
>   src/usermetadata.cpp 51c87f8 
> 
> Diff: https://git.reviewboard.kde.org/r/126595/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126170: [OS X] make kded5 an agent, and build it as a regular application instead of an app bundle

2016-01-02 Thread René J . V . Bertin


> On Dec. 2, 2015, 8:51 a.m., David Faure wrote:
> > Please kind in mind that kded must be able to pop up dialogs, though.
> > (cookie dialog, SSL cert messagebox + dialog, etc. etc.).
> > 
> > If making it an "agent" doesn't prevent it from showing GUI elements now 
> > and then, then no problem.
> 
> René J.V. Bertin wrote:
> With the earlier approach of setting `LSUIElement` that is guaranteed to 
> be the case.
> 
> I just checked; launching Qt's Assistant with 
> `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM=1` all that changes is that 
> the application remains in the background; it can be brought into the 
> foreground, and it retains its presence in the Dock and app switcher.
> 
> IOW, I'm not really sure I understand why kded5 doesn't retain that 
> presence with `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM` set. It's 
> possible that all the env. variable does is postpone the actions that lead to 
> that presence. If that's true than we'd have to come back to the more 
> appropriate previous revision of this patch.
> 
> OTOH: the only dialogs I have seen under KDE4 that are related to kded 
> (unknown cert) were posted when kded4 was *not* running. Ditto for cookie 
> related things. Under what circumstances is kded supposed to present a GUI?
> 
> David Faure wrote:
> Here is an easy way to test this: do the same change for kiod in kio 
> (it's like a mini kded) and then
> cd kio/tests ; ./listjobtest ftp://t...@upload.kde.org
> should bring up a password dialog.
> 
> Except that with Qt 5.6 from git here (from some time ago) it asserts in 
> DBus (looking into that now)... but hopefully you have Qt 5.5 ?
> 
> René J.V. Bertin wrote:
> OK, here's a reason NOT to use 
> QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM: it has to be unset, and I'm 
> not sure when this would have to/could be done such that the variable is 
> picked up for kded5 itself, but not by anything that is launched subsequently.
> 
> If kded5 is used to start kdeinit5 for instances, everything launched 
> through there will launch in the background.
> 
> So I'm going to go back to the original proposition, because an 
> LSUIElement app is exactly what kded ought to be.
> 
> David Faure wrote:
> I don't understand what you're saying. kded5 isn't starting any other 
> processes, is it?
> 
> René J.V. Bertin wrote:
> kdeinit5 is auto-started as part of what I think is normal behaviour. So 
> if kded5 is started first, that's what starts kdeinit5. Shouldn't it?
> 
> My reasoning here is that users might be interested in the possibility to 
> prepare the KDE runtime infrastructure with an inoffensive and non-invasive 
> daemon process that is part of the infrastructure itself. It is my experience 
> with KDE4/Mac that not doing so, but leaving the bootstrapping until you 
> start some larger and (much) more complex application (or suite; think 
> starting Akonadi) can lead to subtle but annoying stability issues.
> 
> NB: starting kded5 through kdeinit5 does *not* work on OS X. I've had a 
> quick look to understand why, but quickly gave up due to the complexity of 
> the code.
> 
> David Faure wrote:
> If a user wants to prepare the runtime infrastructure, he/she should run 
> kdeinit5, not kded5.
> kdeinit5 is the master of everything; kded is just a bunch of on-demand 
> services.
> 
> Apps started standalone, who then make a dbus call to kded might indeed 
> start kded first, which would in turn start kdeinit.
> But yeah, unset any env vars in kdeinit that shouldn't be set for the 
> apps it starts, that makes sense.

> If a user wants to prepare the runtime infrastructure, he/she should run 
> kdeinit5, not kded5.

The thing with that is that s/he would then have to launch 2 applications.

> Apps started standalone, who then make a dbus call to kded might indeed start 
> kded first

If that is also how `kdeinit5 --kded` starts kded, then that won't work. The 
KDE daemon has always been tricky on OS X, and it just works best in practice 
to let that application be the KDE bootstrap utility. We have to take into 
consideration the fact that the session dbus itself is started asynchronously 
through launchd, which makes relying on its presence (and being operational) in 
order to launch other services tricky at best.

A "bunch of on-demand services" itself started on explicit user demand and 
which activates the master of everything KDE when that's necessary (without 
relying on the session dbus) ... what is wrong with the KDE Daemon being the 
master puppet player like that, instead of startked on full Plasma systems?


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126170/#review89022
---


On Dec. 25, 2015, 10:14 p.m., René J.V. 

Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 12:50:59 David Faure wrote:

> > I added a "bring application to the foreground" call (to a KWindowSystem 
> > helper function) in there, which alleviated the issue of applications 
> > remaining somewhere in the background on OS X. 
> > I'm still keeping an eye out for central locations where I could call that 
> > helper function.
> 
> This would break focus-stealing-prevention.
> Apps started while the user is typing elsewhere shouldn't be brought to the 
> foreground.

Yeah, well, there's a fine line to walk there. It's not like it never happens 
on Linux either. In fact, it happens much more often on Linux than with native 
OS X applications, even if OS X didn't always do so well. You can (again) start 
an app (via Dock or Finder) and then switch back immediately to whatever other 
app you were using, and the new app will open just behind the one you switched 
back to.
I *think* that this is something LaunchServices takes care of.

Of course that all doesn't apply to popup messages, password dialogs and the 
like. Hence the fine line remark: I think everyone is used to focus being 
stolen in certain circumstances. I won't call it a small price to pay, but you 
can also consider that a "feature" causing new applications to open in the 
background systematically is a form of focus stealing - a more annoying one at 
that because most of the time you don't notice that it's happening.

R
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126170: [OS X] make kded5 an agent, and build it as a regular application instead of an app bundle

2016-01-02 Thread David Faure


> On Dec. 2, 2015, 7:51 a.m., David Faure wrote:
> > Please kind in mind that kded must be able to pop up dialogs, though.
> > (cookie dialog, SSL cert messagebox + dialog, etc. etc.).
> > 
> > If making it an "agent" doesn't prevent it from showing GUI elements now 
> > and then, then no problem.
> 
> René J.V. Bertin wrote:
> With the earlier approach of setting `LSUIElement` that is guaranteed to 
> be the case.
> 
> I just checked; launching Qt's Assistant with 
> `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM=1` all that changes is that 
> the application remains in the background; it can be brought into the 
> foreground, and it retains its presence in the Dock and app switcher.
> 
> IOW, I'm not really sure I understand why kded5 doesn't retain that 
> presence with `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM` set. It's 
> possible that all the env. variable does is postpone the actions that lead to 
> that presence. If that's true than we'd have to come back to the more 
> appropriate previous revision of this patch.
> 
> OTOH: the only dialogs I have seen under KDE4 that are related to kded 
> (unknown cert) were posted when kded4 was *not* running. Ditto for cookie 
> related things. Under what circumstances is kded supposed to present a GUI?
> 
> David Faure wrote:
> Here is an easy way to test this: do the same change for kiod in kio 
> (it's like a mini kded) and then
> cd kio/tests ; ./listjobtest ftp://t...@upload.kde.org
> should bring up a password dialog.
> 
> Except that with Qt 5.6 from git here (from some time ago) it asserts in 
> DBus (looking into that now)... but hopefully you have Qt 5.5 ?
> 
> René J.V. Bertin wrote:
> OK, here's a reason NOT to use 
> QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM: it has to be unset, and I'm 
> not sure when this would have to/could be done such that the variable is 
> picked up for kded5 itself, but not by anything that is launched subsequently.
> 
> If kded5 is used to start kdeinit5 for instances, everything launched 
> through there will launch in the background.
> 
> So I'm going to go back to the original proposition, because an 
> LSUIElement app is exactly what kded ought to be.
> 
> David Faure wrote:
> I don't understand what you're saying. kded5 isn't starting any other 
> processes, is it?
> 
> René J.V. Bertin wrote:
> kdeinit5 is auto-started as part of what I think is normal behaviour. So 
> if kded5 is started first, that's what starts kdeinit5. Shouldn't it?
> 
> My reasoning here is that users might be interested in the possibility to 
> prepare the KDE runtime infrastructure with an inoffensive and non-invasive 
> daemon process that is part of the infrastructure itself. It is my experience 
> with KDE4/Mac that not doing so, but leaving the bootstrapping until you 
> start some larger and (much) more complex application (or suite; think 
> starting Akonadi) can lead to subtle but annoying stability issues.
> 
> NB: starting kded5 through kdeinit5 does *not* work on OS X. I've had a 
> quick look to understand why, but quickly gave up due to the complexity of 
> the code.
> 
> David Faure wrote:
> If a user wants to prepare the runtime infrastructure, he/she should run 
> kdeinit5, not kded5.
> kdeinit5 is the master of everything; kded is just a bunch of on-demand 
> services.
> 
> Apps started standalone, who then make a dbus call to kded might indeed 
> start kded first, which would in turn start kdeinit.
> But yeah, unset any env vars in kdeinit that shouldn't be set for the 
> apps it starts, that makes sense.
> 
> René J.V. Bertin wrote:
> > If a user wants to prepare the runtime infrastructure, he/she should 
> run kdeinit5, not kded5.
> 
> The thing with that is that s/he would then have to launch 2 applications.
> 
> > Apps started standalone, who then make a dbus call to kded might indeed 
> start kded first
> 
> If that is also how `kdeinit5 --kded` starts kded, then that won't work. 
> The KDE daemon has always been tricky on OS X, and it just works best in 
> practice to let that application be the KDE bootstrap utility. We have to 
> take into consideration the fact that the session dbus itself is started 
> asynchronously through launchd, which makes relying on its presence (and 
> being operational) in order to launch other services tricky at best.
> 
> A "bunch of on-demand services" itself started on explicit user demand 
> and which activates the master of everything KDE when that's necessary 
> (without relying on the session dbus) ... what is wrong with the KDE Daemon 
> being the master puppet player like that, instead of startked on full Plasma 
> systems?

Users don't have to start kded by hand, it's dbus-activated when apps need it. 
Starting kdeinit is enough. In theory it's all autostarted, but I had to start 
kdeinit before ctest for insta

Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread David Faure
On Saturday 02 January 2016 13:46:17 René J.V. Bertin wrote:
> On Saturday January 02 2016 12:50:59 David Faure wrote:
> 
> > > I added a "bring application to the foreground" call (to a KWindowSystem 
> > > helper function) in there, which alleviated the issue of applications 
> > > remaining somewhere in the background on OS X. 
> > > I'm still keeping an eye out for central locations where I could call 
> > > that helper function.
> > 
> > This would break focus-stealing-prevention.
> > Apps started while the user is typing elsewhere shouldn't be brought to the 
> > foreground.
> 
> Yeah, well, there's a fine line to walk there. It's not like it never happens 
> on Linux either. In fact, it happens much more often on Linux than with 
> native OS X applications, even if OS X didn't always do so well. You can 
> (again) start an app (via Dock or Finder) and then switch back immediately to 
> whatever other app you were using, and the new app will open just behind the 
> one you switched back to.
> I *think* that this is something LaunchServices takes care of.
> 
> Of course that all doesn't apply to popup messages, password dialogs and the 
> like. Hence the fine line remark: I think everyone is used to focus being 
> stolen in certain circumstances. I won't call it a small price to pay, but 
> you can also consider that a "feature" causing new applications to open in 
> the background systematically is a form of focus stealing - a more annoying 
> one at that because most of the time you don't notice that it's happening.

Sounds to me like you're saying "better bring all apps to front than none".

Maybe, maybe not, but what we need to continue this discussion is hard data.
1) what does QProcess on Mac do (always in front, always in background, depends 
on whether the user is typing)
2) can QProcess be improved
3) can we use QProcess from KF5 instead of kdeinit (because, again, kdeinit's 
only purpose is fork+exec
which is apparently not a good solution on Mac). I'm pretty sure the answer to 
this one is yes.
E.g. if you set bool useKToolInvocation to false in krun.cpp:724, you'll get 
into the code path that uses QProcess.
(which btw shows another reason to ensure QProcess works: kdeinit isn't even 
always used, even on Linux,
see the conditional for useKToolInvocation).

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


test failures on http://ci-logs.kde.flaska.net

2016-01-02 Thread David Faure
On Saturday 07 November 2015 21:42:33 Jan Kundrát wrote:
> 
> > Can you the value of 
> > QMimeDatabase().mimeTypeForName("text/plain").preferredSuffix()
> > on your system? Here's it's "txt", I suspect it's "doc" on your 
> > system. Not sure why yet
> > though, but let's first check that.

Ah, I found the issue.

2016-01-01 20:36:41,206 [output] FAIL!  : KNewFileMenuTest::test(text file with 
jpeg extension) Compared values are not the same
2016-01-01 20:36:41,206 [output]Actual   (emittedUrl.toLocalFile()): 
"/tmp/knewfilemenutest-oYNW2E/foo.jpg.doc"
2016-01-01 20:36:41,206 [output]Expected (path): 
"/tmp/knewfilemenutest-oYNW2E/foo.jpg.txt"

I can reproduce this if I set XDG_DATA_DIRS to 
/usr/share:/d/kde/inst/kde_frameworks/share:/usr/share
instead of
/d/kde/inst/kde_frameworks/share:/usr/share
(as it normally is on my system).

And indeed the value of XDG_DATA_DIRS in
http://ci-logs.kde.flaska.net/7f/7f50569d59eabc20897f54fb97483bd7f8b89d63/rebuilddep/rebuilddep-kf5-qt55-clang-el7/3cef5be/shell_output.log
is long and messy and has /usr/share in front (so it's viewed as being "on top 
of" the kde mimetype file, while it's supposed to be the other way around).

Can this be fixed in the CI setup?

Thanks.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126170: [OS X] make kded5 an agent, and build it as a regular application instead of an app bundle

2016-01-02 Thread René J . V . Bertin


> On Dec. 2, 2015, 8:51 a.m., David Faure wrote:
> > Please kind in mind that kded must be able to pop up dialogs, though.
> > (cookie dialog, SSL cert messagebox + dialog, etc. etc.).
> > 
> > If making it an "agent" doesn't prevent it from showing GUI elements now 
> > and then, then no problem.
> 
> René J.V. Bertin wrote:
> With the earlier approach of setting `LSUIElement` that is guaranteed to 
> be the case.
> 
> I just checked; launching Qt's Assistant with 
> `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM=1` all that changes is that 
> the application remains in the background; it can be brought into the 
> foreground, and it retains its presence in the Dock and app switcher.
> 
> IOW, I'm not really sure I understand why kded5 doesn't retain that 
> presence with `QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM` set. It's 
> possible that all the env. variable does is postpone the actions that lead to 
> that presence. If that's true than we'd have to come back to the more 
> appropriate previous revision of this patch.
> 
> OTOH: the only dialogs I have seen under KDE4 that are related to kded 
> (unknown cert) were posted when kded4 was *not* running. Ditto for cookie 
> related things. Under what circumstances is kded supposed to present a GUI?
> 
> David Faure wrote:
> Here is an easy way to test this: do the same change for kiod in kio 
> (it's like a mini kded) and then
> cd kio/tests ; ./listjobtest ftp://t...@upload.kde.org
> should bring up a password dialog.
> 
> Except that with Qt 5.6 from git here (from some time ago) it asserts in 
> DBus (looking into that now)... but hopefully you have Qt 5.5 ?
> 
> René J.V. Bertin wrote:
> OK, here's a reason NOT to use 
> QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM: it has to be unset, and I'm 
> not sure when this would have to/could be done such that the variable is 
> picked up for kded5 itself, but not by anything that is launched subsequently.
> 
> If kded5 is used to start kdeinit5 for instances, everything launched 
> through there will launch in the background.
> 
> So I'm going to go back to the original proposition, because an 
> LSUIElement app is exactly what kded ought to be.
> 
> David Faure wrote:
> I don't understand what you're saying. kded5 isn't starting any other 
> processes, is it?
> 
> René J.V. Bertin wrote:
> kdeinit5 is auto-started as part of what I think is normal behaviour. So 
> if kded5 is started first, that's what starts kdeinit5. Shouldn't it?
> 
> My reasoning here is that users might be interested in the possibility to 
> prepare the KDE runtime infrastructure with an inoffensive and non-invasive 
> daemon process that is part of the infrastructure itself. It is my experience 
> with KDE4/Mac that not doing so, but leaving the bootstrapping until you 
> start some larger and (much) more complex application (or suite; think 
> starting Akonadi) can lead to subtle but annoying stability issues.
> 
> NB: starting kded5 through kdeinit5 does *not* work on OS X. I've had a 
> quick look to understand why, but quickly gave up due to the complexity of 
> the code.
> 
> David Faure wrote:
> If a user wants to prepare the runtime infrastructure, he/she should run 
> kdeinit5, not kded5.
> kdeinit5 is the master of everything; kded is just a bunch of on-demand 
> services.
> 
> Apps started standalone, who then make a dbus call to kded might indeed 
> start kded first, which would in turn start kdeinit.
> But yeah, unset any env vars in kdeinit that shouldn't be set for the 
> apps it starts, that makes sense.
> 
> René J.V. Bertin wrote:
> > If a user wants to prepare the runtime infrastructure, he/she should 
> run kdeinit5, not kded5.
> 
> The thing with that is that s/he would then have to launch 2 applications.
> 
> > Apps started standalone, who then make a dbus call to kded might indeed 
> start kded first
> 
> If that is also how `kdeinit5 --kded` starts kded, then that won't work. 
> The KDE daemon has always been tricky on OS X, and it just works best in 
> practice to let that application be the KDE bootstrap utility. We have to 
> take into consideration the fact that the session dbus itself is started 
> asynchronously through launchd, which makes relying on its presence (and 
> being operational) in order to launch other services tricky at best.
> 
> A "bunch of on-demand services" itself started on explicit user demand 
> and which activates the master of everything KDE when that's necessary 
> (without relying on the session dbus) ... what is wrong with the KDE Daemon 
> being the master puppet player like that, instead of startked on full Plasma 
> systems?
> 
> David Faure wrote:
> Users don't have to start kded by hand, it's dbus-activated when apps 
> need it. Starting kdeinit is enough. In theory it's all autostarted, but I 
> had to sta

Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 14:14:19 David Faure wrote:

>Sounds to me like you're saying "better bring all apps to front than none".

Yes, but also that I think that practice should be more subtle than that if we 
manage to use LaunchServices consistently where it's appropriate.

Disregarding of course the fact that the QCocoaIntegration ctor calls the same 
AppKit function that my KWindowSystem helper function uses, if 
QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM isn't set, Which may or may not 
be enough, I do not yet have sufficient data on that.
But not that if that bit of code does what it seems to be doing, Qt5 
applications are indeed always opened in front, and the cases I'm seeing where 
this isn't the case correspond in fact to GUIs posted by applications that were 
already running.

>Maybe, maybe not, but what we need to continue this discussion is hard data.
>1) what does QProcess on Mac do (always in front, always in background, 
>depends on whether the user is typing)

You're probably better aware (or able to understand) than I exactly what 
QProcess does behind the scenes. I can affirm that it is basically common 
knowledge that native GUI applications that are started via a call from the 
exec() family or through system() will open in the layer behind the one 
occupied by the parent process.

I don't think the data will get any harder than that, unless we get our hands 
on the sources of the relevant SDKs.

>2) can QProcess be improved

It should be possible, as long as there are no hard feature requirements that 
are incompatible with the use of LaunchServices. 

>3) can we use QProcess from KF5 instead of kdeinit (because, again, kdeinit's 
>only purpose is fork+exec
>which is apparently not a good solution on Mac). I'm pretty sure the answer to 
>this one is yes.

Yes, but I'm not sure there is an advantage to using QProcess if you only use 
it to "detach" a new application with which you're not keeping any ties. I 
don't think QProcess allows much more than that, other than stopping and 
killing the spawned application (but I should check again). If that's all true, 
I don't see why we would wait for a hypothetical QProcess improvement, instead 
of using a native API directly, at least for GUI applications that live in app 
bundles.

IOW, if a launch request comes in for "foo.app" or "/path/to/foo.app" or 
"/path/to/foo.app/Contents/MacOS/foo-bundle-exec", use the LaunchServices SDK 
to start "foo.app" with whatever arguments have to be passed in. Otherwise, use 
QProcess (indeed, why not).
The biggest hurdle to using native SDKs (coding in ObjC) has already been 
taken, after all.

>E.g. if you set bool useKToolInvocation to false in krun.cpp:724, you'll get 
>into the code path that uses QProcess.
>(which btw shows another reason to ensure QProcess works: kdeinit isn't even 
>always used, even on Linux,
>see the conditional for useKToolInvocation).

Yeah, klaunch does that too (or something similar), I noticed. OTOH, if we 
streamline kinit_mac.mm so that its launch() function does something like what 
I outlined above, why would we stick with useKToolInvocation=false. At the 
least we could put the native code in a common code file that's included by 
kdeinit and klaunch and family, if we want to avoid the overhead of relaying a 
launch request via kdeinit.

In fact, someone else already asked if there's any point in maintaining kdeinit 
on a platform where it's main function (caching shared libraries) is moot. Is 
there?

R.
>
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4support

2016-01-02 Thread Michael Pyne

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126507/
---

(Updated Jan. 2, 2016, 4:02 p.m.)


Review request for KDE Frameworks.


Changes
---

Remove the double-lookup.

Agreed on the relative safety of the GUI thread. The big issue I had when first 
prepping this patch was reconciling object lifetime of `*tiles` when tiles is 
return value of `QCache::object()` vs. when use `QCache::insert()`. If we make 
local copy of `*tiles` then we must remember to free it. If we use return value 
of `::object()` we must *not* free it.

I also had a version of this patch that just uses a bool flag `shouldDelete` 
and just uses the existing `*tiles` throughout, which might be simpler. I'll 
leave it up to you whether you think that would be a better fit here.


Repository: kdelibs4support


Description
---

Fix a couple of Coverity issues:

1. CID 1175508; file descriptors used in KLockFile could be leaked in
error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl
also checks that the lock has been taken, which is only considered true
if LockOK had been returned in our lock function. Instead close() the fd
ourselves unless we make it to LockOK.

2. CID 117; The standard mis-use of QCache. QCache::insert can, in
theory, delete our object as soon as we insert it into cache, so we have
to check for that. Even ::contains() and ::object() can be risky (the
pointers returned by object() have no lifetime guarantee), but since
this is GUI code I assume it's only used single-threaded and not
re-entrant. Otherwise we'd need even more paranoia...


Diffs (updated)
-

  src/kdecore/klockfile_unix.cpp 67283a5 
  src/kdeui/k4style.cpp a1a2ab1 

Diff: https://git.reviewboard.kde.org/r/126507/diff/


Testing
---

Everything builds and appears to still work, though it's hard to test K4Style 
as I'm not sure what uses it right at this point.


Thanks,

Michael Pyne

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread David Faure
On Saturday 02 January 2016 15:59:04 René J.V. Bertin wrote:
> On Saturday January 02 2016 14:14:19 David Faure wrote:
> >Maybe, maybe not, but what we need to continue this discussion is hard data.
> >1) what does QProcess on Mac do (always in front, always in background, 
> >depends on whether the user is typing)
> 
> You're probably better aware (or able to understand) than I exactly what 
> QProcess does behind the scenes. I can affirm that it is basically common 
> knowledge that native GUI applications that are started via a call from the 
> exec() family or through system() will open in the layer behind the one 
> occupied by the parent process.
> I don't think the data will get any harder than that, unless we get our hands 
> on the sources of the relevant SDKs.

While I like the general approach of reading source code, what I meant here was 
*testing*.
If it behaves like you want it to behave, that's perfect, no need to understand 
every line of its code to assess that.

> >2) can QProcess be improved
> 
> It should be possible, as long as there are no hard feature requirements that 
> are incompatible with the use of LaunchServices. 
> 
> >3) can we use QProcess from KF5 instead of kdeinit (because, again, 
> >kdeinit's only purpose is fork+exec
> >which is apparently not a good solution on Mac). I'm pretty sure the answer 
> >to this one is yes.
> 
> Yes, but I'm not sure there is an advantage to using QProcess if you only use 
> it to "detach" a new application with which you're not keeping any ties. I 
> don't think QProcess allows much more than that, other than stopping and 
> killing the spawned application (but I should check again). 

Sounds right, for QProcess::startDetached()  (no stdin/stdout/stderr). But the 
question is, how does it behave in terms
of bringing the new app to the background or foreground.

> If that's all true, I don't see why we would wait for a hypothetical QProcess 
> improvement, instead of using a native API directly, at least for GUI 
> applications that live in app bundles.

I very much disagree with this approach. Qt is opensource, if there's something 
to be fixed in QProcess, make a patch,
then it won't be hypothetical. I don't see "starting a GUI application" as a 
KF5-specific need at all.

> >E.g. if you set bool useKToolInvocation to false in krun.cpp:724, you'll get 
> >into the code path that uses QProcess.
> >(which btw shows another reason to ensure QProcess works: kdeinit isn't even 
> >always used, even on Linux,
> >see the conditional for useKToolInvocation).
> 
> Yeah, klaunch does that too (or something similar), I noticed. OTOH, if we 
> streamline kinit_mac.mm so that its launch() function does something like 
> what I outlined above, why would we stick with useKToolInvocation=false. At 
> the least we could put the native code in a common code file that's included 
> by kdeinit and klaunch and family, if we want to avoid the overhead of 
> relaying a launch request via kdeinit.

You missed my point. Whatever you do to kdeinit, when KRun uses the 
"useKToolInvocation==false" code path,
then QProcess will be used anyway, so you have an interest in making that work 
correctly.

As the boolean condition says, this is the case when tempFiles==true (the app 
is responsible for deleting the file after use)
or when runService is called with a "fake" KService (created by code, not in 
ksycoca)
or when suggestedFileName is set (comes from HTTP content-disposition).

But QProcess is used in many other places too (e.g. dropjob when dropping a 
file onto an exe in a filemanager).

> In fact, someone else already asked if there's any point in maintaining 
> kdeinit on a platform where it's main function (caching shared libraries) is 
> moot. Is there?

For starting apps, no, there isn't, that's what I keep telling you

The other use of kdeinit, starting kioslaves in a way that they can be passed 
from a process to another,
still remains. Therefore my recommendation is: don't kill kdeinit, keep it for 
kioslaves, but you don't need
it to start apps. Hence my suggestion of useKToolInvocation=false in krun.cpp, 
and the few other places that
use ktoolinvocation (to talk to klauncher, to talk to kdeinit).

The alternative is indeed to implement app launching in klauncher itself, but 
you still need the other ways
to start apps (anywhere QProcess is used) to work correctly anyway. So why not 
use QProcess everywhere?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4support

2016-01-02 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126507/#review90467
---

Ship it!


Ship It!

- David Faure


On Jan. 2, 2016, 4:02 p.m., Michael Pyne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126507/
> ---
> 
> (Updated Jan. 2, 2016, 4:02 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> Fix a couple of Coverity issues:
> 
> 1. CID 1175508; file descriptors used in KLockFile could be leaked in
> error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl
> also checks that the lock has been taken, which is only considered true
> if LockOK had been returned in our lock function. Instead close() the fd
> ourselves unless we make it to LockOK.
> 
> 2. CID 117; The standard mis-use of QCache. QCache::insert can, in
> theory, delete our object as soon as we insert it into cache, so we have
> to check for that. Even ::contains() and ::object() can be risky (the
> pointers returned by object() have no lifetime guarantee), but since
> this is GUI code I assume it's only used single-threaded and not
> re-entrant. Otherwise we'd need even more paranoia...
> 
> 
> Diffs
> -
> 
>   src/kdecore/klockfile_unix.cpp 67283a5 
>   src/kdeui/k4style.cpp a1a2ab1 
> 
> Diff: https://git.reviewboard.kde.org/r/126507/diff/
> 
> 
> Testing
> ---
> 
> Everything builds and appears to still work, though it's hard to test K4Style 
> as I'm not sure what uses it right at this point.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126028: Add support for desktopFileName to NETWinInfo

2016-01-02 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126028/#review90472
---

Ship it!


Sorry, forgot to review this earlier.

The @since version needs to be updated (to 5.19 actually, since I just tagged 
5.18). Unless you shout quickly for a 5.18 respin.

- David Faure


On Nov. 11, 2015, 1:15 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126028/
> ---
> 
> (Updated Nov. 11, 2015, 1:15 p.m.)
> 
> 
> Review request for KDE Frameworks, kwin and David Faure.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Implementation for the new hint _NET_WM_DESKTOP_FILE, see [1].
> Till it's fully standardized going with a KDE prefix, so it's
> _KDE_NET_WM_DESKTOP_FILE. Once it's specified the atom name can
> be exchanged.
> 
> [1] https://mail.gnome.org/archives/wm-spec-list/2015-November/msg0.html
> 
> 
> Diffs
> -
> 
>   autotests/netwininfotestclient.cpp 59a3980ff589fc3de9e479905c191bcbf1747644 
>   src/netwm_def.h 0938e2b28f6b0d425a16748b52a5b8e4704d8af6 
>   src/platforms/xcb/atoms_p.h b5a6e7efc98ff8033c0854041428a7d4b52ffc93 
>   src/platforms/xcb/netwm.h 220340bf0e96d2a186b72e118b601471529aeabf 
>   src/platforms/xcb/netwm.cpp 2335c297bb627065ca9b7e691290bfbdc64bccc3 
>   src/platforms/xcb/netwm_p.h e0645bbfd2c2061d9201fe34160c6201d89f4a89 
> 
> Diff: https://git.reviewboard.kde.org/r/126028/diff/
> 
> 
> Testing
> ---
> 
> tested with a modified kcmshell5 and a modified desktop file.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: closest equivalent to KApplicationPrivate::init() ?

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 17:14:07 David Faure wrote:

> While I like the general approach of reading source code, what I meant here 
> was *testing*.
> If it behaves like you want it to behave, that's perfect, no need to 
> understand every line of its code to assess that.

OK, hard *empirical* data (though that 2nd sentence reads like a very 
qualitative and subjective version ;) )

Anyway, I'll be gathering that kind of data from now on.

> Sounds right, for QProcess::startDetached()  (no stdin/stdout/stderr). But 
> the question is, how does it behave in terms
> of bringing the new app to the background or foreground.

My recent experience allows me to be pretty damn sure that it indeed lets the 
application stay in the background.

> I very much disagree with this approach. Qt is opensource, if there's 
> something to be fixed in QProcess, make a patch,
> then it won't be hypothetical. I don't see "starting a GUI application" as a 
> KF5-specific need at all.

You saw I said "wait for QProcess to be improved", right? I don't expect a 
patch for QProcess to be incorporated before 5.7.x ...

> You missed my point. Whatever you do to kdeinit, when KRun uses the 
> "useKToolInvocation==false" code path,
> then QProcess will be used anyway, so you have an interest in making that 
> work correctly.

Ah, right. I missed that indeed.

> to start apps (anywhere QProcess is used) to work correctly anyway. So why 
> not use QProcess everywhere?

Ok, so what about the requirements on QProcess? Anything beyond what's possible 
with QProcess::startDetached and possibly launch confirmation? Should QProcess 
figure out transparently (or rather, opaquely :)) whether to use LaunchServices 
or not, or should the developer be given a way to override it, or even switch 
to switch to LaunchServices rather than using standard Posix APIs?

R
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread René J . V . Bertin
Hi,


I have gotten a bit further. As I thought it is critical to have a correct 
Info.plist (or else the event is never sent), and it isn't trivial to edit that 
file "in place" and have the system recognise its new content (easiest way is 
to copy the entire app bundle in the Finder).

You also need to set a flag (LSMultipleInstancesProhibited) in the Info.plist 
to avoid launching multiple copies; for some reason the mechanism that is in 
place to prevent that doesn't work when you drop a document on Kate's app icon 
in the Finder (or the Dock). So:

 NSPrincipalClass
 NSApplication
 NSSupportsAutomaticGraphicsSwitching
 
 LSMultipleInstancesProhibited
 
 CFBundleDocumentTypes
 
  
   CFBundleTypeExtensions
   
*
   
   CFBundleTypeName
   NSStringPboardType
   CFBundleTypeRole
   Editor
  
 

(The NSSupportsAutomaticGraphicsSwitching comes from QtCreator; I haven't check 
what it does exactly but it shouldn't harm).

With this, I see the events arrive in kate's main.cpp when I add the following:

#include 
class FileOpenHandler : public QObject
{
Q_OBJECT
public:
FileOpenHandler(QObject *parent=Q_NULLPTR)
: QObject(parent)
{}
bool eventFilter(QObject *obj, QEvent *event)
{
if (event->type() == QEvent::FileOpen) {
QFileOpenEvent *foe = static_cast(event);
qDebug() << Q_FUNC_INFO << "FileOpen event" << foe;
// call KateApp::openUrl(foe->url() ...) from here
return true;
} else {
return QObject::eventFilter(obj, event);
}
}
};

and then just before starting the main loop:

#ifdef Q_OS_OSX
FileOpenHandler *fileOpenHandler = new FileOpenHandler(qApp);
qApp->installEventFilter(fileOpenHandler);
#endif

Question is: what about the encoding parameter?

Also, for a local/MacPorts KDE4 implementation of this : if I add a signal to 
the KApplication class, do I need to rebuild all dependents to avoid 
ABI-related crashing, or only those that are modified to connect to the new 
signal?
(IOW, can you add a signal to a class without breaking backwards compatibility?)


R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread Christoph Cullmann
Hi,

> Hi,
> 
> 
> I have gotten a bit further. As I thought it is critical to have a correct
> Info.plist (or else the event is never sent), and it isn't trivial to edit 
> that
> file "in place" and have the system recognise its new content (easiest way is
> to copy the entire app bundle in the Finder).
> 
> You also need to set a flag (LSMultipleInstancesProhibited) in the Info.plist 
> to
> avoid launching multiple copies; for some reason the mechanism that is in 
> place
> to prevent that doesn't work when you drop a document on Kate's app icon in 
> the
> Finder (or the Dock). So:
> 
> NSPrincipalClass
> NSApplication
> NSSupportsAutomaticGraphicsSwitching
> 
> LSMultipleInstancesProhibited
> 
> CFBundleDocumentTypes
> 
>  
>   CFBundleTypeExtensions
>   
>*
>   
>   CFBundleTypeName
>   NSStringPboardType
>   CFBundleTypeRole
>   Editor
>  
> 
> 
> (The NSSupportsAutomaticGraphicsSwitching comes from QtCreator; I haven't 
> check
> what it does exactly but it shouldn't harm).
Thanks for the hint with the plist file!

> 
> With this, I see the events arrive in kate's main.cpp when I add the 
> following:
> 
> #include 
> class FileOpenHandler : public QObject
> {
>Q_OBJECT
> public:
>FileOpenHandler(QObject *parent=Q_NULLPTR)
>: QObject(parent)
>{}
>bool eventFilter(QObject *obj, QEvent *event)
>{
>if (event->type() == QEvent::FileOpen) {
>QFileOpenEvent *foe = static_cast(event);
>qDebug() << Q_FUNC_INFO << "FileOpen event" << foe;
>// call KateApp::openUrl(foe->url() ...) from here
>return true;
>} else {
>return QObject::eventFilter(obj, event);
>}
>}
> };
> 
> and then just before starting the main loop:
> 
> #ifdef Q_OS_OSX
>FileOpenHandler *fileOpenHandler = new FileOpenHandler(qApp);
>qApp->installEventFilter(fileOpenHandler);
> #endif
I have added this now just to KateApp.

https://quickgit.kde.org/?p=kate.git&a=commit&h=cd6ec201725cf627a336015d472c39f5ff73b2a7

> 
> Question is: what about the encoding parameter?
Can be just set to empty QString like done in my commit above.

> 
> Also, for a local/MacPorts KDE4 implementation of this : if I add a signal to
> the KApplication class, do I need to rebuild all dependents to avoid
> ABI-related crashing, or only those that are modified to connect to the new
> signal?
> (IOW, can you add a signal to a class without breaking backwards 
> compatibility?)
Signal adding should be binary compatible, but I doubt that is a good idea.

Why not just add the matching code to the applications you want to have support?
They will anyway need adjustments.

Greetings
Christoph

-- 
- Dr.-Ing. Christoph Cullmann -
AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
Science Park 1 Tel:   +49-681-38360-22
66123 Saarbrücken  Fax:   +49-681-38360-20
GERMANYWWW:   http://www.AbsInt.com

Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 19:56:28 Christoph Cullmann wrote:

Hi,


> I have added this now just to KateApp.
> 
> https://quickgit.kde.org/?p=kate.git&a=commit&h=cd6ec201725cf627a336015d472c39f5ff73b2a7

Hmm, thanks for the hint with the Info.plist? You mean you already knew about 
the rest? :)

> Can be just set to empty QString like done in my commit above.

That's what I hoped, but now I can just grab your patch.

> Why not just add the matching code to the applications you want to have 
> support?
> They will anyway need adjustments.

Yes, but it'd be a bit easier if you can just connect to a signal, especially 
if you already have a slot that takes a QUrl to open. KDE4 has its own 
QApplication derivative that can act as the "delegate" for the event; I find 
that a more elegant solution if it can be implemented without breaking binary 
compatibility.

Cheers,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-01-02 Thread Pino Toscano

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126610/
---

Review request for KDE Frameworks.


Repository: kitemviews


Description
---

When indexes are removed and their widgets deleted, the event filter on each 
widget is not removed, leading to the "you should not delete widgets 
manually"-alike warning.

Add an helper forgetAbout() function which performs all the actions done 
per-widget before deleting each, additionally removing also the event filter.


Diffs
-

  src/kwidgetitemdelegate.cpp 779dc2a8a57148fb37f1f5a7194bc9656cb305a4 
  src/kwidgetitemdelegatepool.cpp e916dddad8be56bb803e241da43d8cbe7a171ec3 
  src/kwidgetitemdelegatepool_p.h 401fe193b0954d6c7c721503d4657b7f08e9fd2e 

Diff: https://git.reviewboard.kde.org/r/126610/diff/


Testing
---

A sample application with widgets for items in the model, removing indexes: no 
more warning at removal time.


Thanks,

Pino Toscano

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread Christoph Cullmann
Hi,

> On Saturday January 02 2016 19:56:28 Christoph Cullmann wrote:
> 
> Hi,
> 
> 
>> I have added this now just to KateApp.
>> 
>> https://quickgit.kde.org/?p=kate.git&a=commit&h=cd6ec201725cf627a336015d472c39f5ff73b2a7
> 
> Hmm, thanks for the hint with the Info.plist? You mean you already knew about
> the rest? :)
Yeah, as said, I just never had time to implement it (and was too dumb to get 
MacOS to trigger
the event).

Besides: No need for ifdefs, QFileOpenEvent is there on all platforms, just 
without function.

> 
>> Can be just set to empty QString like done in my commit above.
> 
> That's what I hoped, but now I can just grab your patch.
;) Added the same to KWrite, both should be fine now, at least for the cases
where we have some url.

> 
>> Why not just add the matching code to the applications you want to have 
>> support?
>> They will anyway need adjustments.
> 
> Yes, but it'd be a bit easier if you can just connect to a signal, especially 
> if
> you already have a slot that takes a QUrl to open. KDE4 has its own
> QApplication derivative that can act as the "delegate" for the event; I find
> that a more elegant solution if it can be implemented without breaking binary
> compatibility.
But you are aware that KDE 4.x is in pure maintainance mode and for KF5 based 
stuff
the simplest solution will be the event filter (or adding ::event overwrite if 
the application
already has inherited from qapp)

Greetings
Christoph

-- 
- Dr.-Ing. Christoph Cullmann -
AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
Science Park 1 Tel:   +49-681-38360-22
66123 Saarbrücken  Fax:   +49-681-38360-20
GERMANYWWW:   http://www.AbsInt.com

Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120139: kill warning

2016-01-02 Thread Pino Toscano

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120139/#review90476
---


IMHO the warning is not totally pointless, and it should be better debugged why 
it is triggered.

Anyway, I just posted https://git.reviewboard.kde.org/r/126610/ to avoid 
triggering it on manual index removal.

- Pino Toscano


On Set. 11, 2014, 10:48 a.m., Sune Vuorela wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120139/
> ---
> 
> (Updated Set. 11, 2014, 10:48 a.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Hartmetz and Stephen Kelly.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> The warninng is even triggered by from internal KWidgetItemDelegatePrivate, 
> so seems to be wrong
> 
> Try out the kwidgetitemdelegate test application
> 
> 
> Diffs
> -
> 
>   src/kwidgetitemdelegatepool.cpp d61802e 
> 
> Diff: https://git.reviewboard.kde.org/r/120139/diff/
> 
> 
> Testing
> ---
> 
> Warning not printed.
> 
> 
> Thanks,
> 
> Sune Vuorela
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-01-02 Thread Sune Vuorela

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126610/#review90477
---


Looks much better than my attempt over in 
https://git.reviewboard.kde.org/r/120139

- Sune Vuorela


On Jan. 2, 2016, 7:24 p.m., Pino Toscano wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126610/
> ---
> 
> (Updated Jan. 2, 2016, 7:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> When indexes are removed and their widgets deleted, the event filter on each 
> widget is not removed, leading to the "you should not delete widgets 
> manually"-alike warning.
> 
> Add an helper forgetAbout() function which performs all the actions done 
> per-widget before deleting each, additionally removing also the event filter.
> 
> 
> Diffs
> -
> 
>   src/kwidgetitemdelegate.cpp 779dc2a8a57148fb37f1f5a7194bc9656cb305a4 
>   src/kwidgetitemdelegatepool.cpp e916dddad8be56bb803e241da43d8cbe7a171ec3 
>   src/kwidgetitemdelegatepool_p.h 401fe193b0954d6c7c721503d4657b7f08e9fd2e 
> 
> Diff: https://git.reviewboard.kde.org/r/126610/diff/
> 
> 
> Testing
> ---
> 
> A sample application with widgets for items in the model, removing indexes: 
> no more warning at removal time.
> 
> 
> Thanks,
> 
> Pino Toscano
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120139: kill warning

2016-01-02 Thread Sune Vuorela

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120139/
---

(Updated Jan. 2, 2016, 7:32 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks, Andreas Hartmetz and Stephen Kelly.


Repository: kitemviews


Description
---

The warninng is even triggered by from internal KWidgetItemDelegatePrivate, so 
seems to be wrong

Try out the kwidgetitemdelegate test application


Diffs
-

  src/kwidgetitemdelegatepool.cpp d61802e 

Diff: https://git.reviewboard.kde.org/r/120139/diff/


Testing
---

Warning not printed.


Thanks,

Sune Vuorela

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 20:24:25 Christoph Cullmann wrote:

> Besides: No need for ifdefs, QFileOpenEvent is there on all platforms, just 
> without function.

True, but there's no point in installing an event filter for it on platforms 
where the event never occurs.


> But you are aware that KDE 4.x is in pure maintainance mode and for KF5 based 
> stuff

I know, but it's not going anywhere anytime soon on client desktops and in 
MacPorts; not as long as Qt4 builds I reckon.
Heck, I'll probably be sticking to KDevelop4 on my older hardware if what the 
KDevelop guys say about their new clang-based parser is true (and I believe it; 
even on my i7 earlier versions of that parser were *much* slower than the 
legacy C++ parser).

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread Christoph Cullmann
Hi,

> On Saturday January 02 2016 20:24:25 Christoph Cullmann wrote:
> 
>> Besides: No need for ifdefs, QFileOpenEvent is there on all platforms, just
>> without function.
> 
> True, but there's no point in installing an event filter for it on platforms
> where the event never occurs.
That small overhead doesn't hurt and we don't have yet an other code path that
is only compiled on mac os.

> 
> 
>> But you are aware that KDE 4.x is in pure maintainance mode and for KF5 based
>> stuff
> 
> I know, but it's not going anywhere anytime soon on client desktops and in
> MacPorts; not as long as Qt4 builds I reckon.
> Heck, I'll probably be sticking to KDevelop4 on my older hardware if what the
> KDevelop guys say about their new clang-based parser is true (and I believe 
> it;
> even on my i7 earlier versions of that parser were *much* slower than the
> legacy C++ parser).
I personally wouldn't want to stick with stuff that is not actively developed
and fixed anymore, but you are right, it won't go away that fast.

But keep in mind e.g. for Kate/KWrite, no KDE 4.x based non-critical (aka 
dataloss)
bugs will be fixed anymore, we even have not enough manpower to keep all master
bugs in check :/

Greetings
Christoph

-- 
- Dr.-Ing. Christoph Cullmann -
AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
Science Park 1 Tel:   +49-681-38360-22
66123 Saarbrücken  Fax:   +49-681-38360-20
GERMANYWWW:   http://www.AbsInt.com

Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] new year's resolution: opening files "the Mac way" (TM)

2016-01-02 Thread René J . V . Bertin
On Saturday January 02 2016 20:24:25 Christoph Cullmann wrote:

Hi,

Something else: is the KateApp supposed to be subclassable? In that case you 
may want to put the FileOpen event back into the queue rather than eating it 
(i.e. return false instead of true from the event filter), so that the 
descendant class can receive it too.

If events work that way of course, I've never tried.

> That small overhead doesn't hurt and we don't have yet an other code path
> that is only compiled on mac os.

Agreed, not in a text editor, no.

> I personally wouldn't want to stick with stuff that is not actively
> developed and fixed anymore

Heh, if it ain't broke, don't look for ways to fix it ... at least not beyond 
what's reasonable ;)
The big advantage of using stuff like this (and that works well enough to get 
the job done) is that you actually get around to getting jobs done rather than 
spending a significant amount of time applying updates (or worse, building 
them).
I've already decided that I'm not going to upgrade my ports in sync with the 
monthly release cycle but instead wait until there's a sufficiently long list 
of fixes, new features etc. to warrant the overhead.

Don't worry about me submitting patches for any of this in KDE4. I may decide 
to publish some of the ones for KDELibs via RRs but that's about it.

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126610: kwidgetitemdelegate: properly cleanup widgets on index removal

2016-01-02 Thread Pino Toscano


> On Gen. 2, 2016, 7:31 p.m., Sune Vuorela wrote:
> > Looks much better than my attempt over in 
> > https://git.reviewboard.kde.org/r/120139

Note my patch does not remove the warnings on close, although I found out how 
to avoid them: to the KWidgetItemDelegate construr, either pass a null parent, 
or the view->viewport().


- Pino


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126610/#review90477
---


On Gen. 2, 2016, 7:24 p.m., Pino Toscano wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126610/
> ---
> 
> (Updated Gen. 2, 2016, 7:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> When indexes are removed and their widgets deleted, the event filter on each 
> widget is not removed, leading to the "you should not delete widgets 
> manually"-alike warning.
> 
> Add an helper forgetAbout() function which performs all the actions done 
> per-widget before deleting each, additionally removing also the event filter.
> 
> 
> Diffs
> -
> 
>   src/kwidgetitemdelegate.cpp 779dc2a8a57148fb37f1f5a7194bc9656cb305a4 
>   src/kwidgetitemdelegatepool.cpp e916dddad8be56bb803e241da43d8cbe7a171ec3 
>   src/kwidgetitemdelegatepool_p.h 401fe193b0954d6c7c721503d4657b7f08e9fd2e 
> 
> Diff: https://git.reviewboard.kde.org/r/126610/diff/
> 
> 
> Testing
> ---
> 
> A sample application with widgets for items in the model, removing indexes: 
> no more warning at removal time.
> 
> 
> Thanks,
> 
> Pino Toscano
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: [KDE/Mac] "offending" menu items

2016-01-02 Thread René J . V . Bertin
Hi again,

Remember those 2 menu items that caused repeating error messages on the 
terminal? Turns out that whatever it is that causes it exactly can in fact make 
the application crash (I saw that with Konsole), but I also found a workaround.
Someone could have told me that this kind of action isn't added to menus 
explicitly in the source code, but through an rc file. No wonder I couldn't 
find the lines to disable in C++ code :)

So rather than disabling the Bookmarks and Clipboard History menus altogether 
in KTextEditor, one can decide not to put them into the context menu in 
katepart5ui.rc . This approach proved successful with konsole, I'm not yet sure 
it will too with KTextEditor (first impression is no, but that doesn't seem to 
make sense).

I'll leave it to you and/or other KTextEditor/Kate maintainers to decide if 
this is indeed a workaround, an acceptable one, if either or both of these 
actions would be better in the context menu and not in the main menubar, and of 
course how to deploy it.

Cheers,
R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126474: Port QRegExp to QRegularExpression in kshorturifilter

2016-01-02 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126474/
---

(Updated Jan. 2, 2016, 10:32 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 6763f7a6cab41fc4c94be783305ac2c8e85ccaa1 by David 
Edmundson to branch master.


Repository: kio


Description
---

A static QRegExp was used but it is not thread safe. QRegularExpression
seems to be.

BUG: 352356


Diffs
-

  src/urifilters/shorturi/kshorturifilter.cpp 
6002ec6925c0acdd20a053f98baca46863f69fa6 

Diff: https://git.reviewboard.kde.org/r/126474/diff/


Testing
---

I ran the autotests which includes urifilter and I've run krunner which uses it 
extensively.


Thanks,

David Edmundson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4support

2016-01-02 Thread Michael Pyne

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126507/
---

(Updated Jan. 2, 2016, 11:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit f4e9dbf2b4ee2770e554c735b7604637e7b5ec54 by Michael Pyne 
to branch master.


Repository: kdelibs4support


Description
---

Fix a couple of Coverity issues:

1. CID 1175508; file descriptors used in KLockFile could be leaked in
error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl
also checks that the lock has been taken, which is only considered true
if LockOK had been returned in our lock function. Instead close() the fd
ourselves unless we make it to LockOK.

2. CID 117; The standard mis-use of QCache. QCache::insert can, in
theory, delete our object as soon as we insert it into cache, so we have
to check for that. Even ::contains() and ::object() can be risky (the
pointers returned by object() have no lifetime guarantee), but since
this is GUI code I assume it's only used single-threaded and not
re-entrant. Otherwise we'd need even more paranoia...


Diffs
-

  src/kdecore/klockfile_unix.cpp 67283a5 
  src/kdeui/k4style.cpp a1a2ab1 

Diff: https://git.reviewboard.kde.org/r/126507/diff/


Testing
---

Everything builds and appears to still work, though it's hard to test K4Style 
as I'm not sure what uses it right at this point.


Thanks,

Michael Pyne

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel