D10081: Don't assert if used incorrectly from dbus

[Martin Flöser]

This revision was automatically updated to reflect the committed changes.
Closed by commit R268:7d272782a55e: Dont assert if used incorrectly from 
dbus (authored by graesslin).

REPOSITORY
  R268 KGlobalAccel

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10081?vs=26098=28562

REVISION DETAIL
  https://phabricator.kde.org/D10081

AFFECTED FILES
  src/runtime/component.cpp

To: graesslin, #frameworks, apol, romangg
Cc: romangg, michaelh

D10235: [server] Add support for the frame semantics of Pointer version 5

[Martin Flöser]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:b6bd28ab0492: [server] Add support for the frame 
semantics of Pointer version 5 (authored by graesslin).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D10235?vs=26336=28563#toc

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10235?vs=26336=28563

REVISION DETAIL
  https://phabricator.kde.org/D10235

AFFECTED FILES
  autotests/client/test_wayland_seat.cpp
  src/client/pointer.cpp
  src/client/pointer.h
  src/client/registry.cpp
  src/server/pointer_interface.cpp
  src/server/pointer_interface_p.h
  src/server/seat_interface.cpp
  src/server/seat_interface_p.h

To: graesslin, #kwin, #plasma, #frameworks, romangg
Cc: romangg, plasma-devel, schernikov, michaelh, ZrenBot, alexeymin, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D10993: Fix protocol selection in KUrlNavigator

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> aleksejshilin wrote in kurlnavigator.cpp:349
> > Is this in order to get smb:// instead of smb: ?
> 
> Yes.
> 
> > It looks correct, but not for "file",
> 
> Why not? If I read RFC 8089 correctly, for host rule it references RFC 3986 
> which allows it to be empty, i.e. an empty authority is valid.
> 
> > And I could add a comment like "we want smb:// or ftp://, not just smb: or 
> > ftp:".
> 
> Makes sense, indeed. Will do shortly.

Well, OK, it's not that it's *incorrect* for file:, it just seems unnecessary 
to make that method call. So given that we have an if() already, why not only 
do it in the else?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10993

To: aleksejshilin, #frameworks, dfaure
Cc: michaelh

D11011: Do not assume layout of msghdr and iovec structure

[A . Wilcox]

awilcox added a comment.


  I can confirm that this patch does fix the build on musl.  Thank you!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

KDE CI: Frameworks kglobalaccel kf5-qt5 WindowsMSVCQt5.10 - Build # 5 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kglobalaccel%20kf5-qt5%20WindowsMSVCQt5.10/5/
 Project:
Frameworks kglobalaccel kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 04 Mar 2018 08:46:27 +
 Build duration:
16 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: TestSuite.kglobalshortcuttest

D11011: Do not assume layout of msghdr and iovec structure

[A . Wilcox]

awilcox accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

D11011: Do not assume layout of msghdr and iovec structure

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  In D11011#217980 , @awilcox wrote:
  
  > I can confirm that this patch does fix the build on musl.  Thank you!
  
  
  I will push this after David's review. I hope it's not a problem?

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.10 - Build # 34 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.10/34/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 04 Mar 2018 08:49:09 +
 Build duration:
11 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 42 test(s), Skipped: 0 test(s), Total: 42 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report57%
(4/7)91%
(222/243)91%
(222/243)85%
(23798/27958)53%
(11684/22234)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client100%
(39/39)100%
(39/39)99%
(6/11185)50%
(7649/15263)autotests.server100%
(5/5)100%
(5/5)99%
(359/362)49%
(220/446)src.client99%
(68/69)99%
(68/69)85%
(5590/6544)66%
(1760/2676)src.server99%
(110/111)99%
(110/111)87%
(6733/7717)65%
(2055/3170)src.tools0%
(0/2)0%
(0/2)0%
(0/779)0%
(0/346)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/120)0%
(0/16)tests0%
(0/14)0%
(0/14)0%
(0/1251)0%
(0/317)

D11011: Do not assume layout of msghdr and iovec structure

[A . Wilcox]

awilcox added a comment.


  That's fine with me.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.9 - Build # 5 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.9/5/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 04 Mar 2018 08:49:10 +
 Build duration:
15 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 41 test(s), Skipped: 0 test(s), Total: 42 test(s)Failed: TestSuite.kwayland-testWaylandSurface
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report57%
(4/7)91%
(221/243)91%
(221/243)82%
(22790/27929)50%
(11087/22234)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client97%
(38/39)97%
(38/39)93%
(10354/11182)47%
(7138/15263)autotests.server100%
(5/5)100%
(5/5)99%
(359/362)49%
(220/446)src.client99%
(68/69)99%
(68/69)84%
(5494/6532)65%
(1735/2676)src.server99%
(110/111)99%
(110/111)85%
(6583/7703)63%
(1994/3170)src.tools0%
(0/2)0%
(0/2)0%
(0/779)0%
(0/346)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/120)0%
(0/16)tests0%
(0/14)0%
(0/14)0%
(0/1251)0%
(0/317)

D11011: Do not assume layout of msghdr and iovec structure

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> sharefd_p.h:85
> +msg.msg_iovlen = 1;
> +msg.msg_control = _buf;
> +msg.msg_controllen = sizeof cmsg_buf;

Looks good, it's just a bit inconsistent that iov_base points to io_buf 
(without &) and msg_control points to _buf (with &), both members being 
char [N]. It turns out that both work, but why not use the same syntax both 
times?

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

D11013: Remove trailing data in the protocol parameter in findProtocol

[Jaime Torres Amate]

jtamate created this revision.
jtamate added reviewers: Frameworks, dfaure.
Restricted Application added a project: Frameworks.
jtamate requested review of this revision.

REVISION SUMMARY
  The paramter can receive things like:
  
  - smb:///
  - zip:/home/user/Download/.../Encoder.php
  
  Remove : and whatever comes after it, because the protocols hash doesn't
  have trailing :

TEST PLAN
  kf5.kio.core: Refilling KProtocolInfoFactory cache in the hope to find ""
  
  It is, in my tests, called by KProtocolInfo::icon "" but I'm unable to know 
  where that url is formed.

REPOSITORY
  R241 KIO

BRANCH
  kate1 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11013

AFFECTED FILES
  src/core/kprotocolinfofactory.cpp

To: jtamate, #frameworks, dfaure
Cc: michaelh

D10989: Check for nullptr in indexForNode

[Jaime Torres Amate]

jtamate added a dependency: D11013: Remove trailing data in the protocol 
parameter in findProtocol.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10989

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11013: Remove trailing data in the protocol parameter in findProtocol

[Jaime Torres Amate]

jtamate added a dependent revision: D10989: Check for nullptr in indexForNode.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11013

To: jtamate, #frameworks, dfaure
Cc: michaelh

KDE CI: Frameworks kwayland kf5-qt5 FreeBSDQt5.9 - Build # 30 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20FreeBSDQt5.9/30/
 Project:
Frameworks kwayland kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 04 Mar 2018 08:49:09 +
 Build duration:
57 min and counting
   JUnit Tests
  Name: (root) Failed: 13 test(s), Passed: 24 test(s), Skipped: 0 test(s), Total: 37 test(s)Failed: TestSuite.kwayland-testCompositorFailed: TestSuite.kwayland-testDataDeviceFailed: TestSuite.kwayland-testDataSourceFailed: TestSuite.kwayland-testRegionFailed: TestSuite.kwayland-testShmPoolFailed: TestSuite.kwayland-testSubCompositorFailed: TestSuite.kwayland-testSubSurfaceFailed: TestSuite.kwayland-testWaylandConnectionThreadFailed: TestSuite.kwayland-testWaylandRegistryFailed: TestSuite.kwayland-testWaylandServerDisplayFailed: TestSuite.kwayland-testWaylandShellFailed: TestSuite.kwayland-testWaylandSurfaceFailed: TestSuite.kwayland-testXdgShellV6

D10641: Revoke temporary authorization of KIO slave before sending status to IdleSlave

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> chinmoyr wrote in slavebase.cpp:147
> Now I feel stupid for doing this. Apart from that I am not even confident 
> about this change because it involves changes in KAuth (Obviously I am not 
> sure about them either). How about we drop this and the related revisions for 
> now and just delete the slave whenever there's authorization?

If by delete you mean kill, I think you're right, it's probably simpler and 
safer, and this should happen rarely enough for this not to be a performance 
issue.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10641

To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh

KDE CI: Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.9 - Build # 29 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kdelibs4support%20kf5-qt5%20FreeBSDQt5.9/29/
 Project:
Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 04 Mar 2018 11:21:34 +
 Build duration:
5 min 37 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 38 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kstandarddirstest

D11013: Remove trailing data in the protocol parameter in findProtocol

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  It's called protocol, it's supposed to be just the protocol. The bug is in 
the caller.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11013

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11010: Add MSG_SLAVE_STATUS_V2 to slave interface

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11010

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10993: Fix protocol selection in KUrlNavigator

[Алексей Шилин]

aleksejshilin added inline comments.

INLINE COMMENTS

> dfaure wrote in kurlnavigator.cpp:349
> Well, OK, it's not that it's *incorrect* for file:, it just seems unnecessary 
> to make that method call. So given that we have an if() already, why not only 
> do it in the else?

> Well, OK, it's not that it's *incorrect* for file:, it just seems unnecessary 
> to make that method call.

Ah, I misunderstood what you meant.

> So given that we have an if() already, why not only do it in the else?

Done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10993

To: aleksejshilin, #frameworks, dfaure
Cc: michaelh

D10993: Fix protocol selection in KUrlNavigator

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  fix_kurlnavigator_protocol_selection

REVISION DETAIL
  https://phabricator.kde.org/D10993

To: aleksejshilin, #frameworks, dfaure
Cc: michaelh

D11011: Do not assume layout of msghdr and iovec structure

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 28573.
chinmoyr added a comment.


  used &

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11011?vs=28551=28573

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

AFFECTED FILES
  src/ioslaves/file/sharefd_p.h

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

D10641: Revoke temporary authorization of KIO slave before sending status to IdleSlave

[Chinmoy Ranjan Pradhan]

chinmoyr added inline comments.

INLINE COMMENTS

> dfaure wrote in slavebase.cpp:147
> That looks like horrible API ;)
> A getter that optionally makes changes, based on a boolean... urgh ;)
> 
> "Duplicating" a for loop isn't really duplication, go for a different method.

Now I feel stupid for doing this. Apart from that I am not even confident about 
this change because it involves changes in KAuth (Obviously I am not sure about 
them either). How about we drop this and the related revisions for now and just 
delete the slave whenever there's authorization?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10641

To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh

D10927: Apply fixes to simple clazy warnings

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R284 KCompletion

BRANCH
  clazyfixes

REVISION DETAIL
  https://phabricator.kde.org/D10927

To: fabiank, dfaure
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 136 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/136/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 04 Mar 2018 10:41:18 +
 Build duration:
11 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 56 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

D10824: Delete IdleSlave having temporary authorization

[Chinmoy Ranjan Pradhan]

chinmoyr added inline comments.

INLINE COMMENTS

> dfaure wrote in klauncher.cpp:1114
> This deletes the Slave C++ class, but it doesn't actually kill the ioslave.
> So why do it?
> 
> I'm confused now. Do you want to kill the ioslave (then call slave->kill())
> or do you want to reuse that ioslave, just without any of the previous 
> authorizations (which is what I thought you were doing in slaveStatus()) ?
> If the latter, then why the delete here?

I meant to kill the slave and not reuse it. 
And IdleSlave subclasses QObject and doesn't have kill() which is why I used 
deleteLater which actually kills the IdleSlave.

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

[Chinmoy Ranjan Pradhan]

chinmoyr added inline comments.

INLINE COMMENTS

> ossi wrote in fdreceiver.cpp:62
> any particular reason not to use QFile::remove() here as well?

I didn't want to include QFile. That's the only reason.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10410

To: chinmoyr, #frameworks, ossi, dfaure
Cc: michaelh

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

[Chinmoy Ranjan Pradhan]

chinmoyr added inline comments.

INLINE COMMENTS

> ossi wrote in fdreceiver.cpp:62
> uhm, you're still using it in this very line, just differently ;)

*head-desk* 
I think I have to get my eyes checked.  I will change it right away.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10410

To: chinmoyr, #frameworks, ossi, dfaure
Cc: michaelh

D10824: Delete IdleSlave having temporary authorization

[David Faure]

dfaure added a comment.


  No, that deletes the IdleSlave QObject, but the slave process is still 
running then.
  
IdleSlave::~IdleSlave()
{
}
  
  Not much happening there ;)
  
  Yes, that makes me wondering if the SLAVE_MAX_IDLE thing actually works.
  
  Hmm, and KLauncher::requestHoldSlave looks like it's leaking the IdleSlave 
(well, child of "this", but the object won't be deleted until logging out).
  Unless I'm missing something, that's a bug from 2001 :-)
  
  Anyhow, one thing at a time, can you test if the kioslave process really goes 
away?
  My bet is that it doesn't, in which case you need a call to 
KIOPrivate::sendTerminateSignal(slave->pid());
  
(if that's not available from this repository, add a method 
IdleSlave::kill())
  
  [The alternative solution is to call SlaveBase::exit() from the slave process 
itself, but there's no good place for doing that I think, so better kill it 
when klauncher decides it's idle, it's simpler than sending it a command to ask 
it to exit.]

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[David Faure]

dfaure added a comment.


  Can someone explain to me how switching from pointers to values is making 
anything faster, or is a first step towards making anything faster? This step 
in itself can only make things slower due to more "copying" (refcount updating).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10742

To: jtamate, #frameworks, dfaure
Cc: markg, michaelh

D10273: Create proper SocketAddress

[Oswald Buddenhagen]

ossi added inline comments.

INLINE COMMENTS

> dfaure wrote in fdsender.cpp:24
> In that case I don't understand why SocketAddress takes a QByteArray and not 
> a std::string Because ossi suggested it to unify the API and avoid one 
> conversion to std::string in fdereceiver? But then we have two contradictory 
> goals, we need to decide whether we use Qt or not in fdsender and therefore 
> in SocketAddress. Since one is not supposed to use Qt API without a 
> QCoreApplication instance, and since it's not recommented to run Qt code as 
> root, I think your original idea made sense, no Qt in fdsender nor in 
> SocketAddress.

i can get behind the idea of keeping SocketAddress and FdSender qt-free. it's 
ugly that FdReceiver stands out, but it would be way harder to make that one 
qt-free due to its use of the event loop.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10273

To: chinmoyr, #frameworks, dfaure
Cc: ossi, thiago, dfaure, michaelh

D10409: In linux don't use abstract socket to share file descriptor

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  @ossi I think the changes in file_unix.cpp from D9966 
 also belong here(?)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10822: Store temporary authorization status in IdleSlave

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks (I just realized we need a similar comment next to the enum value 
MSG_SLAVE_STATUS though)

REPOSITORY
  R241 KIO

BRANCH
  D10822

REVISION DETAIL
  https://phabricator.kde.org/D10822

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[Mark Gaiser]

markg added a comment.


  In D10742#218171 , @dfaure wrote:
  
  > Can someone explain to me how switching from pointers to values is making 
anything faster, or is a first step towards making anything faster? This step 
in itself can only make things slower due to more "copying" (refcount updating).
  
  
  I don't know what the ultimate goal of @jtamate is here (speeding things up, 
that i do know).
  But in general, putting something on the stack (aka, no new) is measurable 
faster. For small objects and in small quantities that doesn't matter much. But 
for large objects (KFileItem is large) and in large quantities (also fitting 
for KFileItem) it could very well be a nice speedup!
  
  The other side of this optimizing story is copies where you don't intent them 
to happen but they just do. For instance, a std::vector allocates a 
bunch ahead of time and then copies it in. Or moves since my move semantics 
commits and where applicable. If this were a std::vector you'd had 
no such copy or move.
  
  Back on this patch. It might allow faster routes, but it really depends on 
@jtamate next plans here if the current patch is worth it.
  Sounds like we need more info :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10742

To: jtamate, #frameworks, dfaure
Cc: markg, michaelh

D10993: Fix protocol selection in KUrlNavigator

[Алексей Шилин]

aleksejshilin updated this revision to Diff 28571.
aleksejshilin added a comment.


  - Move setting authority to the else branch

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10993?vs=28519=28571

BRANCH
  fix_kurlnavigator_protocol_selection

REVISION DETAIL
  https://phabricator.kde.org/D10993

AFFECTED FILES
  src/filewidgets/kurlnavigator.cpp

To: aleksejshilin, #frameworks, dfaure
Cc: michaelh

D10411: Create socket file in user's runtime directory

[David Faure]

dfaure added a comment.


  That doesn't look like an atomic commit. In this commit, "path" changes from 
relative to absolute, but the caller hasn't been updated to actually send an 
absolute path?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10411

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10410

To: chinmoyr, #frameworks, ossi, dfaure
Cc: michaelh

D10567: Remove handling of privilege operation confirmation prompts from KIO::Job

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  D10567

REVISION DETAIL
  https://phabricator.kde.org/D10567

To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh

D10641: Revoke temporary authorization of KIO slave before sending status to IdleSlave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> slavebase.cpp:147
>  
> -bool hasTempAuth()
> +bool hasTempAuth(bool revoke = false)
>  {

That looks like horrible API ;)
A getter that optionally makes changes, based on a boolean... urgh ;)

"Duplicating" a for loop isn't really duplication, go for a different method.

> slavebase.cpp:554
>  qint8 b = connected ? 1 : 0;
> -KIO_DATA << pid << mProtocol << host << b << d->hasTempAuth();
> +KIO_DATA << pid << mProtocol << host << b << d->hasTempAuth(true);
>  if (d->onHold) {

It turns out that indeed this method is only called when the slave is going to 
Idle, but, hmm, in this code nothing says so, it looks like a method that just 
sends current status, and that could be called at any moment...

I would suggest to at least add a comment here, something like
// This method is only called from the IdleSlave constructor, the slave has 
just been returned to klauncher, clear any authorization so that another 
application cannot benefit from it.

... above the method call that you'll have to add to clear the auths, due to 
the previous comment ;)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10641

To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh

D10824: Delete IdleSlave having temporary authorization

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> klauncher.cpp:1114
> +mSlaveList.removeAll(slave);
> +slave->deleteLater();
> +}

This deletes the Slave C++ class, but it doesn't actually kill the ioslave.
So why do it?

I'm confused now. Do you want to kill the ioslave (then call slave->kill())
or do you want to reuse that ioslave, just without any of the previous 
authorizations (which is what I thought you were doing in slaveStatus()) ?
If the latter, then why the delete here?

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 8 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/8/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 04 Mar 2018 10:57:18 +
 Build duration:
17 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(296/444)67%
(296/444)53%
(31614/59621)38%
(18556/48870)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8565/9113)48%
(5229/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)59%
(8412/14347)50%
(4903/9722)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3897/7874)33%
(1644/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(511/982)41%
(412/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1783/4338)35%
(1375/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1333)55%
(648/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%
(8/12)52%
 

KDE CI: Frameworks breeze-icons kf5-qt5 WindowsMSVCQt5.10 - Build # 8 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20breeze-icons%20kf5-qt5%20WindowsMSVCQt5.10/8/
 Project:
Frameworks breeze-icons kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 04 Mar 2018 11:06:39 +
 Build duration:
8 min 36 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

[Oswald Buddenhagen]

ossi added inline comments.

INLINE COMMENTS

> fdreceiver.cpp:62
>  }
> +::unlink(QFile::encodeName(m_path));
>  }

any particular reason not to use QFile::remove() here as well?

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10410

To: chinmoyr, #frameworks, ossi, dfaure
Cc: michaelh

D10409: In linux don't use abstract socket to share file descriptor

[Oswald Buddenhagen]

ossi added a comment.


  that contradicts the comments i added to both reviews.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10822: Store temporary authorization status in IdleSlave

[David Faure]

dfaure added a comment.


  Thanks. Can you just add one comment, for the future?

INLINE COMMENTS

> idleslave.cpp:91
> +}
> +} else {
> +if (!stream.atEnd()) {

// compat code for KF < 5.45. TODO KF6: remove

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10822

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10409: In linux don't use abstract socket to share file descriptor

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  Sorry, I meant D10411 .

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

KDE CI: Frameworks kio kf5-qt5 WindowsMSVCQt5.10 - Build # 10 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20WindowsMSVCQt5.10/10/
 Project:
Frameworks kio kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 04 Mar 2018 10:41:18 +
 Build duration:
1 hr 51 min and counting
   JUnit Tests
  Name: (root) Failed: 27 test(s), Passed: 26 test(s), Skipped: 0 test(s), Total: 53 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-ktcpsockettestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiogui-favicontestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-jobguitestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-kdynamicjobtrackernowidgetstestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

D10824: Delete IdleSlave having temporary authorization

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  Actually deleteLater() does kill the slave. I placed a qDebug statement in 
there and immediately after the message I am getting
  
kdeinit5: PID $slave_pid terminated.

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

D10822: Store temporary authorization status in IdleSlave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> idleslave.cpp:70
>  deleteLater();
> -} else if (cmd != MSG_SLAVE_STATUS) {
> +} else if (cmd != MSG_SLAVE_STATUS_V2) {
>  qCritical() << "Unexpected data from KIO slave.";

This could be made more runtime compatible by not rejecting MSG_SLAVE_STATUS 
completely, and handling it below (e.g. with a test on cmd at the right place, 
to still share the local variables).

(That's what I meant by "support both here").

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10822

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10993: Fix protocol selection in KUrlNavigator

[Алексей Шилин]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:37868c2c57f7: Fix protocol selection in KUrlNavigator 
(authored by aleksejshilin).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10993?vs=28571=28574

REVISION DETAIL
  https://phabricator.kde.org/D10993

AFFECTED FILES
  src/filewidgets/kurlnavigator.cpp

To: aleksejshilin, #frameworks, dfaure
Cc: michaelh

D10437: Update file ioslave's temporary authorization list

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  (Reusing the same phab request is fine in this case, because there were no 
review comments yet.
  Once there are, and they don't apply to a completely redesigned commit, then 
indeed better start a new phab request.)
  
  This commit looks ok, but I would merge it with the one that adds the method 
addTemporaryAuthorization(actionId), they go together.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10437

To: chinmoyr, #frameworks, dfaure, fvogt
Cc: markg, anthonyfieroni, michaelh

D11011: Do not assume layout of msghdr and iovec structure

[Chinmoy Ranjan Pradhan]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:2a2a8a0fe5ab: Do not assume layout of msghdr and iovec 
structure (authored by chinmoyr).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11011?vs=28573=28576

REVISION DETAIL
  https://phabricator.kde.org/D11011

AFFECTED FILES
  src/ioslaves/file/sharefd_p.h

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 156 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/156/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 04 Mar 2018 10:41:18 +
 Build duration:
16 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(296/444)67%
(296/444)53%
(31539/59611)38%
(18516/48874)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8565/9113)48%
(5232/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)58%
(8343/14346)50%
(4872/9718)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3896/7874)33%
(1643/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(511/974)41%
(412/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1788/4338)35%
(1373/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(631/1333)55%
(649/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%

D10312: FileUndoManager: don't delete non-existing local files

[David Faure]

dfaure added a comment.


  Looks good, just minor issues left.

INLINE COMMENTS

> fileundomanagertest.cpp:735
> +FileUndoManager::self()->recordCopyJob(copyJob);
> +const bool ok = copyJob->exec();
> +QVERIFY(ok);

This isn't Q_ASSERT, it's ok to perform the operation inside the macro.

  QVERIFY2(job->exec(), qPrintable(job->errorString()));

> fileundomanagertest.cpp:744
> +const bool ok = deleteJob->exec();
> +QVERIFY(ok);
> +QVERIFY(!QFileInfo::exists(dest.toLocalFile()));

same here

> fileundomanagertest.cpp:753
> +FileUndoManager::self()->undo();
> +QCOMPARE(spyUndoAvailable.count(), 1);
> +QVERIFY(!FileUndoManager::self()->undoAvailable());

This is missing a check for the value of the bool emitted.

  QVERIFY(!spyUndoAvailable.at(0).at(0).toBool());

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10312

To: elvisangelaccio, dfaure
Cc: ngraham, #frameworks, michaelh

D10543: fix crashing with duplicate entries

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REVISION DETAIL
  https://phabricator.kde.org/D10543

To: sandsmark, dfaure, #frameworks
Cc: apol, michaelh

D11019: Remove left-over code from kdelibs splitting time

[Friedrich W . H . Kossebau]

kossebau created this revision.
kossebau added a reviewer: dfaure.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
kossebau requested review of this revision.

TEST PLAN
  Building/installing kinit still works, projects using
  kf5_add_kdeinit_executable() also still build

REPOSITORY
  R303 KInit

BRANCH
  removekdelibssplitcode

REVISION DETAIL
  https://phabricator.kde.org/D11019

AFFECTED FILES
  KF5InitMacros.cmake

To: kossebau, dfaure
Cc: #frameworks, michaelh

D10824: Delete IdleSlave having temporary authorization

[Chinmoy Ranjan Pradhan]

chinmoyr added a comment.


  Neither SlaveBase::exit nor Slave::kill is called. I also commented the line 
in ConnectionBackend which removes the socket file but even then the slave 
process terminated. 
  At this point I have no idea why is this working.

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 WindowsMSVCQt5.10 - Build # 11 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20WindowsMSVCQt5.10/11/
 Project:
Frameworks kio kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 04 Mar 2018 12:33:16 +
 Build duration:
1 hr 19 min and counting
   JUnit Tests
  Name: (root) Failed: 24 test(s), Passed: 29 test(s), Skipped: 0 test(s), Total: 53 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-jobguitestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

D11019: Remove left-over code from kdelibs splitting time

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R303 KInit

BRANCH
  removekdelibssplitcode

REVISION DETAIL
  https://phabricator.kde.org/D11019

To: kossebau, dfaure
Cc: #frameworks, michaelh

D11019: Remove left-over code from kdelibs splitting time

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R303:3bcf7a1b3421: Remove left-over code from kdelibs 
splitting time (authored by kossebau).

REPOSITORY
  R303 KInit

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11019?vs=28595=28599

REVISION DETAIL
  https://phabricator.kde.org/D11019

AFFECTED FILES
  KF5InitMacros.cmake

To: kossebau, dfaure
Cc: #frameworks, michaelh

D10820: Send slave's polkit authorization status to the host

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> slavebase.cpp:150
> +#ifdef Q_OS_UNIX
> +QSet::const_iterator it = m_tempAuths.begin();
> +while (it != m_tempAuths.end()) {

This can't be a const_iterator, if you use it for erase()ing. I'm surprised 
this compiles...

> slavebase.cpp:161
> +
> +bool hasTempAuth()
> +{

const

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10820

To: chinmoyr, dfaure
Cc: anthonyfieroni, #frameworks, michaelh

D10409: In linux don't use abstract socket to share file descriptor

[David Faure]

dfaure added a comment.


  Can you explain a bit more in the commit log why this is better? I guess it 
comes from a discussion elsewhere, but better have the info here and in git in 
the end.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10568: Handle privilege operation confirmation prompts in SlaveBase

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Just minor requests

INLINE COMMENTS

> slavebase.cpp:127
> +QString m_warningMessage;
> +bool m_confirmationAsked;
>  int m_privilegeOperationStatus;

move next to the other bool (-> less padding)

> slavebase.cpp:517
>  d->m_rootEntryListed = false;
> +d->m_confirmationAsked = false;
>  d->m_privilegeOperationStatus = OperationNotAllowed;

BTW now that there are 5 duplicated lines below the //reset comment (in error 
and finished), it would be worth extracting a reset function...

> slavebase.cpp:1495
> +if (d->m_privilegeOperationStatus == OperationAllowed) {
> +d->m_privilegeOperationStatus = d->askConfirmation();
>  }

This reads like it's going to ask confirmation every time this method is called 
(once we are in OperationAllowed state).

The method impl uses a bool to ask only once, but that doesn't show here.

One solution is to rename the method to maybeAskConfirmation, but that's not 
great.
Better might be to test the bool here?

  if (d->m_privilegeOperationStatus == OperationAllowed && 
!d->m_confirmationAsked) {
  d->m_confirmationAsked = true;
  d->m_privilegeOperationStatus = d->askConfirmation();
  }

This implies a small behavior change: in your patch, if the user presses 
Cancel, then he might still get asked again, while in my case he wouldn't. But, 
unless I'm wrong, after Cancel we'll go to SlaveBase::error() which will reset 
both member vars anyway, right?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10568

To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh

D10818: Store PolicyKit action which the slave is authorized to perform

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Looks ok, but this commit is a bit too small ;) It would make more sense to 
integrate this with the code that's calling addTemporaryAuthorization, so we 
can see how that all works.

REPOSITORY
  R241 KIO

BRANCH
  D10818

REVISION DETAIL
  https://phabricator.kde.org/D10818

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 7 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/7/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 04 Mar 2018 10:41:18 +
 Build duration:
8 min 19 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(297/444)67%
(297/444)53%
(31837/59629)38%
(18660/48870)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8565/9113)48%
(5231/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)59%
(8415/14347)50%
(4902/9722)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3897/7874)33%
(1644/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(511/974)41%
(412/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1788/4338)35%
(1373/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1333)55%
(648/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%

D11011: Do not assume layout of msghdr and iovec structure

[David Faure]

dfaure accepted this revision.
dfaure added a comment.


  Thanks, this is good to go in. In fact I'll put it in 5.44 since it fixes a 
compilation error.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11011

To: chinmoyr, dfaure, awilcox
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 157 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/157/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 04 Mar 2018 10:57:52 +
 Build duration:
6 min 52 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(296/444)67%
(296/444)53%
(31540/59619)38%
(18520/48874)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8565/9113)48%
(5232/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)58%
(8344/14345)50%
(4868/9718)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3896/7874)33%
(1643/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(511/982)41%
(412/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1783/4338)35%
(1375/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(631/1333)55%
(649/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 137 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/137/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 04 Mar 2018 10:57:18 +
 Build duration:
7 min 17 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 56 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest

KDE CI: Frameworks baloo kf5-qt5 SUSEQt5.10 - Build # 40 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20baloo%20kf5-qt5%20SUSEQt5.10/40/
 Project:
Frameworks baloo kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 04 Mar 2018 11:12:59 +
 Build duration:
9 min 18 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 38 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kinotifytest

KDE CI: Frameworks baloo kf5-qt5 SUSEQt5.9 - Build # 7 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20baloo%20kf5-qt5%20SUSEQt5.9/7/
 Project:
Frameworks baloo kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 04 Mar 2018 11:12:59 +
 Build duration:
8 min 56 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 38 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kinotifytest

D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

[Oswald Buddenhagen]

ossi added inline comments.

INLINE COMMENTS

> chinmoyr wrote in fdreceiver.cpp:62
> I didn't want to include QFile. That's the only reason.

uhm, you're still using it in this very line, just differently ;)

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10410

To: chinmoyr, #frameworks, ossi, dfaure
Cc: michaelh

D9966: [KIO] Fix issues with sharing of file descriptor

[Oswald Buddenhagen]

ossi added a comment.


  looks good, though technically speaking this still fixes two separate issues.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9966

To: chinmoyr, #frameworks, thiago, dfaure, ossi
Cc: ngraham, fvogt, lbeltrame, dfaure, michaelh

D10822: Store temporary authorization status in IdleSlave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 28577.
chinmoyr added a comment.


  Added support for MSG_SLAVE_STATUS

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10822?vs=28543=28577

BRANCH
  D10822

REVISION DETAIL
  https://phabricator.kde.org/D10822

AFFECTED FILES
  src/core/idleslave.cpp
  src/core/idleslave.h

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10409: In linux don't use abstract socket to share file descriptor

[Oswald Buddenhagen]

ossi added a comment.


  the idea is that you can do directory-based access controls on file-based 
sockets, while the abstract namespace has no controls.
  otoh, only linux has the abstract namespace, and it supports peer credential 
verification as well, so this doesn't actually add any security afaict.
  arguably, the patch still makes sense from a maintenance perspective, 
removing a redundant code path.
  fwiw, i'd re-order this patch before the other one - it makes for smaller 
patches to first remove code and then refactor only what's left.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10822: Store temporary authorization status in IdleSlave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 28581.
chinmoyr added a comment.


  added the required comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10822?vs=28577=28581

BRANCH
  D10822

REVISION DETAIL
  https://phabricator.kde.org/D10822

AFFECTED FILES
  src/core/idleslave.cpp
  src/core/idleslave.h

To: chinmoyr, dfaure
Cc: #frameworks, michaelh

D10409: In linux don't use abstract socket to share file descriptor

[Oswald Buddenhagen]

ossi added a comment.


  not sure why; the changes are semantically separate.
  
  my suggestion was to put this before D10273 
, thus reducing the latter's size.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10409

To: chinmoyr, #frameworks, ossi
Cc: dfaure, michaelh

D10824: Delete IdleSlave having temporary authorization

[David Faure]

dfaure added a comment.


  Interesting. But are you sure that neither Slave::kill() nor 
SlaveBase::exit() is called? It can't just be the deleteLater that does this, 
unless I'm missing something (e.g. closing the pipe makes the slave die with 
SIGPIPE maybe)

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

KDE CI: Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.9 - Build # 30 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kdelibs4support%20kf5-qt5%20FreeBSDQt5.9/30/
 Project:
Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 04 Mar 2018 17:33:12 +
 Build duration:
6 min 22 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 38 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kstandarddirstest

D10446: Add KLanguageName

[Albert Astals Cid]

aacid added a comment.


  ping?

REPOSITORY
  R265 KConfigWidgets

REVISION DETAIL
  https://phabricator.kde.org/D10446

To: aacid
Cc: #frameworks, michaelh

D8964: Fix two bugs in KMessageWidget

[Albert Astals Cid]

aacid planned changes to this revision.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D8964

To: aacid, #frameworks
Cc: ngraham, dhaumann, anthonyfieroni, #frameworks, michaelh

D10168: Use nullptr for passing a null pointer to crc32

[Albert Astals Cid]

aacid added a comment.


  ping?

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D10168

To: aacid
Cc: #frameworks, michaelh

D10166: Add -Wlogical-op -Wzero-as-null-pointer-constant to KF5 warnings

[Albert Astals Cid]

aacid planned changes to this revision.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D10166

To: aacid
Cc: cgiboudeaux, dhaumann, #frameworks, #build_system, michaelh

D10824: Delete IdleSlave having temporary authorization

[David Faure]

dfaure added a comment.


  I tested what happens in the current code when an idle job is killed. My 
testcase was configuring to use KIO in componentchooser ("open http urls in an 
application based on the contents on the URL") and then `kioclient5 exec 
www.kde.org`. This puts the slave on hold while it emits the mimetype, while 
KIO runs the associated application for that mimetype.
  The idea was that the app can then use that job to resume the same transfer, 
if the app uses KIO. In my testcase it's launching konqueror+webenginepart, 
which doesn't use KIO, so the slave remains idle and gets killed later by 
klauncher.
  
  What happens just after KLauncher::idleTimeout deletes the slave is that the 
slave goes to this code path inside mimeType() :
  
if (ret == -1) {
qDebug() << "read error";
exit();
}
  
  So in my testcase at least, it goes to SlaveBase::exit(), which is how the 
slave exits :)
  You could try the same set of debugging statements in your testcase... 
http://www.davidfaure.fr/2018/exit_debug.diff for kio and 
http://www.davidfaure.fr/2018/kinit.diff for kinit.
  Remember to restart kdeinit5 afterwards by typing kdeinit5 in a terminal - 
but I guess you know this already ;-)

REPOSITORY
  R303 KInit

REVISION DETAIL
  https://phabricator.kde.org/D10824

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks, michaelh

D10927: Apply fixes to simple clazy warnings

[Fabian Kosmale]

This revision was automatically updated to reflect the committed changes.
Closed by commit R284:29e7a1d89278: Apply fixes to simple clazy warnings 
(authored by fabiank).

REPOSITORY
  R284 KCompletion

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10927?vs=28288=28597

REVISION DETAIL
  https://phabricator.kde.org/D10927

AFFECTED FILES
  src/kcompletion_p.h
  src/khistorycombobox.cpp

To: fabiank, dfaure
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 WindowsMSVCQt5.10 - Build # 12 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20WindowsMSVCQt5.10/12/
 Project:
Frameworks kio kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 04 Mar 2018 20:02:04 +
 Build duration:
1 hr 32 min and counting
   JUnit Tests
  Name: (root) Failed: 25 test(s), Passed: 28 test(s), Skipped: 0 test(s), Total: 53 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-ktcpsockettestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-jobguitestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

D9973: ktooltipwidget: Fix tooltip positioning

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in ktooltippositiontest.cpp:71
> Weird, I usually use `qPrintable()` for this, but
> 
>   QTest::addRow(qPrintable(QStringLiteral("small/%1").arg(i.key(
> 
> still gives me the compiler warning. Not sure what is going on here.

QTest::addRow() requires Qt 5.9.

You probably want newRow() instead, which doesn't have printf-style syntax, so 
it won't give you a printf-style warning.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D9973

To: michaelh, elvisangelaccio, #frameworks, #dolphin, ngraham
Cc: dfaure, cfeck, michaelh

Re: Fallback Icon Theme

[Martin Kostolný]

Thanks, Albert, for your answer and point of view!

We actually need the icons since quite a lot of them is misisng and Krusader 
looks (e.g. with Adwaita) quite unfinished with default configuration. We also 
get bug reports about missing icons.

From your opinions and opinions I've already heard - mixing icon themes is not 
usually preferable and forcing breeze or oxygen is not what user expects or 
wants either. Therefore the best would be allowing a user to configure icon 
theme specifically for Krusader from Krusader's config. And, ideally (in the 
long run), design all the missing non-standard icon images for hicolor theme in 
Krusader.

If there is a better way of solving our situation, please tell us :).

Thanks!
Martin


On sobota 3. března 2018 12:16:17 CET Albert Astals Cid wrote:
> El dijous, 1 de març de 2018, a les 23:37:14 CET, Martin Kostolný va escriure:
> > Hi!
> > 
> > We at Krusader would like to address a rather standard issue of applications
> > - missing icons. And we would like an opinion of you as experts :).
> > 
> > Starter discussion (which led in here) is on Krusader's phabricator:
> > https://phabricator.kde.org/D10352
> > 
> > Krusader is using a lot of icons existing in Breeze or Oxygen themes. But
> > e.g. for Gnome users there are a lot of missing icons. We don't want to add
> > all the non-standard icons to hicolor theme attached to Krusader.
> > Especially since they already exist in Breeze or Oxygen. Instead we would
> > like to specify a fallback theme - e.g. Breeze, so every missing icon from
> > currently set theme would be loaded from the fallback theme (if it is
> > installed of course).
> > 
> > I believe this functionality is currently not supported by KIconLoader but I
> > may be wrong. I have only found a possibility to add an additional path to
> > icons (extraSearchPaths) which is probably not what we want.
> > 
> > To give an example, I'd expect something like this to be a possibility for a
> > programmer:
> > 
> > KIconLoader *iconLoader = KIconLoader::global();
> > QStringList fallbackThemes;
> > fallbackThemes << "breeze-dark";
> > fallbackThemes << "oxygen";
> > iconLoader->setFallbackThemes(fallbackThemes);
> > 
> > Any guidance or another opinion is appreciated. Thanks!
> 
> Yeah, i don't think such functionality exists, some people may even say 
> that's 
> undesirable since it may cause weird visual "identity" on when mixing icon 
> themes.
> 
> Once functionality that does exist is forcing the icon theme (in QIcon), 
> that's also a bit weird since it overrides the user selection which can be 
> seen as a bit aggresive, but if you *really* need the icons it may be a 
> possibility.
> 
> That's the other question, do you really need the icons or is it just a nice 
> to have?
> 
> Cheers,
>   Albert
> 
> > Martin
> 
> 
> 
> 

D9973: ktooltipwidget: Fix tooltip positioning

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ktooltippositiontest.cpp:39-52
> +void KTooltipPositionTest::init()
> +{
> +m_container = new QWidget();
> +m_target = new QLabel(QStringLiteral("dummy file"));
> +QHBoxLayout* layout = new QHBoxLayout(m_container);
> +layout->addWidget(m_target);
> +m_margin = 
> m_container->style()->pixelMetric(QStyle::PM_ToolTipLabelFrameWidth);

Imho these variables could be local to `shouldNotObscureTarget()`, so that we 
don't need to define `init()` and `cleanup()` and we don't need to make them 
class members.

> ktooltippositiontest.cpp:65
> +positions.insert(QStringLiteral("centered")
> +, QPoint(m_screenGeometry.width() / 2, m_screenGeometry.height() / 
> 2));
> +

Weird comma position :p

> ktooltippositiontest.cpp:71
> +//FIXME: Compose names w/o compiler warning -Wformat-security
> +
> QTest::addRow(QStringLiteral("small/%1").arg(i.key()).toLatin1().constData()) 
> +<< i.value()

Weird, I usually use `qPrintable()` for this, but

  QTest::addRow(qPrintable(QStringLiteral("small/%1").arg(i.key(

still gives me the compiler warning. Not sure what is going on here.

> ktooltippositiontest.cpp:85
> +
> +if (m_screenGeometry.width() >= 1600 && m_screenGeometry.width() >= 
> 900) {
> +QTest::addRow(QStringLiteral("super 
> large/%1").arg(i.key()).toLatin1().constData())

Did you mean

  if (m_screenGeometry.width() >= 1600 && m_screenGeometry.height() >= 900) {
...
  }

?

> ktooltippositiontest.cpp:111
> +: m_screenGeometry.right() + position.x() - m_container->width()
> +, position.y() >= 0 
> +? position.y() 

Weird comma position. Maybe two local QPoint variables could also improve the 
readability here.

> ktooltippositiontest.cpp:122
> +
> +QLabel* contentWidget(new QLabel(content));
> +contentWidget->setMaximumSize(m_screenGeometry.width() - 20, 
> m_screenGeometry.height() - 20);

Unusual style, we usually do `QLabel* contentWidget = new QLabel(content);`

Also, where is this label deleted?

> ktooltippositiontest.cpp:125
> +
> +QWindow* transientParent(m_container->windowHandle());
> +KToolTipWidget tooltip;

Same as above about the style.

But are we sure we want to //create// a new QWindow? Why not just pass 
`m_container->windowHandle()` to `showBelow()` ?

Also, what if the transient parent is null? We should probably add a 
`QVERIFY(m_container->windowHandle())`

> ktooltippositiontest.cpp:131
> +
> +//TODO: Remove before landing
> +qDebug() << "target: " << targetRect;

We can actually keep (and maybe improve) this debug output. qDebug() will only 
print something if the test fails.

> ktooltippositiontest.cpp:136
> +QVERIFY2(!tooltipRect.intersects(targetRect), "Target obscured");
> +QVERIFY2(tooltipRect.intersected(m_screenGeometry) == tooltipRect, 
> "Displayed offscreen");
> +

This should be a QCOMPARE

> ktooltippositiontest.cpp:139
> +// Check margins
> +if (tooltipRect.bottom() <  targetRect.top() ) {
> +QCOMPARE(m_margin, targetRect.top() - tooltipRect.bottom());

remove space before )

> ktooltippositiontest.cpp:148
> +} else {
> +QFAIL("Test is wrong");
> +} 

What do you mean with "test is wrong"? Can this branch ever happen?

> ktooltippositiontest.cpp:151
> +
> +tooltip.hide();
> +}

`QVERIFY(tooltip.isHidden());` ?

> ktooltippositiontest.h:38-39
> +
> +void shouldNotObscureTarget_data();
> +void shouldNotObscureTarget();
> +

Is there a reason why we are creating a new test binary only for this test 
function? Why not use `ktooltipwidgettest.cpp`?

> ktooltipwidget.cpp:122
>  // - the content is not drawn inside rect
> -
> -const QSize size = content->sizeHint();
> -const int margin = 
> q->style()->pixelMetric(QStyle::PM_ToolTipLabelFrameWidth);
> +q->adjustSize();
> +const QSize size = q->sizeHint();

`centerBelow()` is `const`, but this actually changes the tooltip, right?
Maybe we can move this call to `addWidget()` ?

> ktooltipwidget.cpp:142
> +// Disallow negative x. Happens when rect is wider than screen.
> +x = qMax(screenGeometry.left(), screenGeometry.right() - 
> size.width() + 1);
>  }

This looks unrelated to this patch. I don't see a testcase for it and if I 
revert this change, the new tests still pass.

Ideally we need a failing test case that is fixed by this line of code. Btw, 
don't we have a similar "negative y" problem?

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D9973

To: michaelh, elvisangelaccio, #frameworks, #dolphin, ngraham
Cc: cfeck, michaelh

Re: Global Dependency and Release Freeze

[Ben Cooksley]

Hi all,

We've now finished catching up on everything and getting the CI system
settled down again so we're now able to lift the freeze.
I'll start sorting out the releases which have been requested shortly.

There is still some lingering breakage impacting Akonadi and KIO
however those can be fixed in isolation and do not affect other
builds.

The only significant breakage remaining is a crash within
update-mime-database which means tests reliant on XDG mimetypes may
fail or behave unusually on Windows.
Compilation is not affected by this.

Cheers,
Ben

D4911: add Baloo DBus signals for moved or removed files

[Matthieu Gallien]

mgallien added a comment.


  In D4911#199527 , @cullmann wrote:
  
  > @cullmann: T7860  lists some of my 
observations wrt samba shares. I don't know NFS, can you enlighten me regarding 
deviceids and inodes on NFS?
  >
  > I can only tell that baloo can't work at all on NFS/SMB home dirs with the 
current DB and that its idea of inode number usage to encode the file won't 
work there either (it already doesn't work on large file systems with 64-bit 
inodes).
  >
  > I am very happy that somebody is working on fixing the current bugs, but I 
am still not sure it would not just be better to port away to some better 
maintained and more secure thing like tracker, but my  
https://cgit.kde.org/clones/baloo/cullmann/tbaloo.git/ port stagnated
  >  as there was no interest.
  
  
  I did some test today on tbaloo and noticed one problem when fetching the 
results from a query. The paths are encoded like URLs but without the scheme. I 
had to modify my code to use it. Apart from that, nice work. It just works. Are 
you still interested to work on that ?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D4911

To: mgallien, vhanda, dfaure, michaelh, #baloo
Cc: ngraham, cullmann, apol, #frameworks, ashaposhnikov, michaelh, spoorun, 
nicolasfella, alexeymin

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 158 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/158/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Mon, 05 Mar 2018 04:15:12 +
 Build duration:
33 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(296/444)67%
(296/444)53%
(31539/59620)38%
(18524/48874)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8563/9113)48%
(5231/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)58%
(8344/14346)50%
(4872/9718)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3896/7874)33%
(1643/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(512/982)41%
(413/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1783/4338)35%
(1375/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(631/1333)55%
(649/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%

D11037: balooctl: checkDb should also verify the last known url for the documentId.

[James Smith]

smithjd created this revision.
smithjd added a reviewer: Baloo.
Restricted Application added projects: Frameworks, Baloo.
Restricted Application added a subscriber: Frameworks.
smithjd requested review of this revision.

REVISION SUMMARY
  Report orphan documentId's.

REPOSITORY
  R293 Baloo

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D11037

AFFECTED FILES
  src/engine/transaction.cpp

To: smithjd, #baloo
Cc: #frameworks, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin

D10446: Add KLanguageName

[Aleix Pol Gonzalez]

apol added a comment.


  Makes sense to me anyway.
  
  Is there any other code that could use this API?

INLINE COMMENTS

> klanguagename.cpp:45
> +
> +return QString();
> +}

shouldn't this be equivalent to nameForCodeInLocale(code, QLocale().code())?

> klanguagename.cpp:65
> +entry.setLocale("en");
> +const QString englisName = group.readEntry("Name");
> +if (name != englisName) {

englishName

REPOSITORY
  R265 KConfigWidgets

REVISION DETAIL
  https://phabricator.kde.org/D10446

To: aacid
Cc: apol, #frameworks, michaelh

D11038: balooctl: Add pruneDb option to remove stale file index entries.

[James Smith]

smithjd created this revision.
smithjd added reviewers: Baloo, Frameworks.
Restricted Application added projects: Frameworks, Baloo.
smithjd requested review of this revision.

REPOSITORY
  R293 Baloo

BRANCH
  master-purgeDb (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11038

AFFECTED FILES
  src/engine/transaction.cpp
  src/engine/transaction.h
  src/tools/balooctl/main.cpp

To: smithjd, #baloo, #frameworks
Cc: ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin

D10997: Fix conversion of AccessPoint flags to capabilities

[Jan Grulich]

jgrulich accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10997

To: fvogt, #frameworks, jgrulich, kfunk
Cc: #frameworks, michaelh

KDE CI: Frameworks kio kf5-qt5 WindowsMSVCQt5.10 - Build # 13 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20WindowsMSVCQt5.10/13/
 Project:
Frameworks kio kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Mon, 05 Mar 2018 04:15:12 +
 Build duration:
6 min 11 sec and counting
   CONSOLE OUTPUT
  Started by an SCM changeRunning in Durability level: MAX_SURVIVABILITY[Pipeline] nodeStill waiting to schedule taskWaiting for next available executor on WindowsMSVCQt5.10Running on Windows Builder 1 in C:\CI\workspace\Frameworks kio kf5-qt5 WindowsMSVCQt5.10[Pipeline] {[Pipeline] timestamps[Pipeline] {[Pipeline] catchError[Pipeline] {[Pipeline] stage[Pipeline] { (Checkout Sources)[Pipeline] deleteDir[Pipeline] }[Pipeline] // stage[Pipeline] }java.nio.file.FileSystemException: C:\CI\workspace\Frameworks kio kf5-qt5 WindowsMSVCQt5.10\build\autotests: The process cannot access the file because it is being used by another process.	at sun.nio.fs.WindowsException.translateToIOException(Unknown Source)	at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)	at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)	at sun.nio.fs.WindowsFileSystemProvider.implDelete(Unknown Source)	at sun.nio.fs.AbstractFileSystemProvider.deleteIfExists(Unknown Source)	at java.nio.file.Files.deleteIfExists(Unknown Source)	at hudson.Util.tryOnceDeleteFile(Util.java:297)	at hudson.Util.deleteFile(Util.java:253)Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from ange.kde.org/144.76.172.7:49686		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1737)		at hudson.remoting.UserResponse.retrieve(UserRequest.java:313)		at hudson.remoting.Channel.call(Channel.java:952)		at hudson.FilePath.act(FilePath.java:998)		at hudson.FilePath.act(FilePath.java:987)		at hudson.FilePath.deleteRecursive(FilePath.java:1192)		at org.jenkinsci.plugins.workflow.steps.DeleteDirStep$Execution.run(DeleteDirStep.java:77)		at org.jenkinsci.plugins.workflow.steps.DeleteDirStep$Execution.run(DeleteDirStep.java:69)		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:49)		at hudson.security.ACL.impersonate(ACL.java:290)		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:46)		at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)		at java.util.concurrent.FutureTask.run(FutureTask.java:266)		at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)		at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)		at java.lang.Thread.run(Thread.java:748)Caused: java.io.IOException: Unable to delete 'C:\CI\workspace\Frameworks kio kf5-qt5 WindowsMSVCQt5.10\build\autotests'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts.	at hudson.Util.deleteFile(Util.java:258)	at hudson.FilePath.deleteRecursive(FilePath.java:1225)	at hudson.FilePath.deleteContentsRecursive(FilePath.java:1234)	at hudson.FilePath.deleteRecursive(FilePath.java:1216)	at hudson.FilePath.deleteContentsRecursive(FilePath.java:1234)	at hudson.FilePath.deleteRecursive(FilePath.java:1216)	at hudson.FilePath.access$1100(FilePath.java:208)	at hudson.FilePath$13.invoke(FilePath.java:1195)	at hudson.FilePath$13.invoke(FilePath.java:1192)	at hudson.FilePath$FileCallableWrapper.call(FilePath.java:2816)	at hudson.remoting.UserRequest.perform(UserRequest.java:210)	at hudson.remoting.UserRequest.perform(UserRequest.java:53)	at hudson.remoting.Request$2.run(Request.java:358)	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)	at java.util.concurrent.FutureTask.run(Unknown Source)	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)	at hudson.remoting.Engine$1$1.run(Engine.java:94)	at java.lang.Thread.run(Unknown Source)Caused: java.io.IOException: remote file operation failed: C:\CI\workspace\Frameworks kio kf5-qt5 WindowsMSVCQt5.10 at hudson.remoting.Channel@28c31b45:JNLP4-connect connection from ange.kde.org/144.76.172.7:49686	at hudson.FilePath.act(FilePath.java:1005)	at hudson.FilePath.act(FilePath.java:987)	at hudson.FilePath.deleteRecursive(FilePath.java:1192)	at org.jenkinsci.plugins.workflow.steps.DeleteDirStep$Execution.run(DeleteDirStep.java:77)	at org.jenkinsci.plugins.workflow.steps.DeleteDirStep$Execution.run(DeleteDirStep.java:69)	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:49)	at hudson.security.ACL.impersonate(ACL.java:290)	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:46)	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)	at java.util.concurrent.FutureTask.run(FutureTask.java:266)	at 

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 138 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/138/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.9
 Date of build:
Mon, 05 Mar 2018 04:15:12 +
 Build duration:
12 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 55 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiowidgets-kdirmodeltest

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 9 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/9/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Mon, 05 Mar 2018 04:15:12 +
 Build duration:
16 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(296/444)67%
(296/444)53%
(31621/59621)38%
(18552/48870)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8565/9113)48%
(5230/10797)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)100%
(5/5)75%
(3/4)src.core84%
(101/120)84%
(101/120)59%
(8415/14347)50%
(4901/9722)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3896/7874)33%
(1643/4942)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(511/982)41%
(412/996)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1788/4338)35%
(1373/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1333)55%
(648/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash67%
(8/12)67%
(8/12)52%