date:20160125

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Martin Gräßlin



> On Jan. 25, 2016, 10:36 p.m., Luca Beltrame wrote:
> > FYI, this breaks Plasma 5 startup completely for me. kinit crashes with
> > 
> > ```gdb
> > #0  0x7ff831e58c20 in QObject::thread() const () at 
> > /usr/lib64/libQt5Core.so.5
> > #1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #4  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:485
> > #5  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:479
> > #6  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (argc=32767, _name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", 
> > cwd=, envc=, envs=, 
> > reset_env=false, tty=0x8e55b0 "", avoid_loops=160, startup_id_str=0x1161 
> > )
> > at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
> > #7  0x008e559d in  ()
> > #8  0x in  ()
> > ```
> > 
> > I had to revert it or kdeinit would crash and prevent me from having a 
> > functional session. ;)
> 
> Andreas Hartmetz wrote:
> Same here.
> 
> Martin Gräßlin wrote:
> sorry about that, I'm on it.
> 
> Martin Gräßlin wrote:
> The problem is that kinit is not a QCoreApplication and it crashes when 
> accessing the ::instance(). A possible workaround for kinit is (not yet 
> tested):
> 
> diff --git a/src/kdeinit/kinit.cpp b/src/kdeinit/kinit.cpp
> index 9919547..8b5c926 100644
> --- a/src/kdeinit/kinit.cpp
> +++ b/src/kdeinit/kinit.cpp
> @@ -375,7 +375,7 @@ static void init_startup_info(KStartupInfoId , 
> const QByteArray ,
>  X11_startup_notify_fd = 
> xcb_get_file_descriptor(s_startup_notify_connection);
>  NETRootInfo rootInfo(s_startup_notify_connection, 
> NET::CurrentDesktop);
>  KStartupInfoData data;
> -data.setDesktop(rootInfo.currentDesktop());
> +data.setDesktop(rootInfo.currentDesktop(true));
>  data.setBin(QFile::decodeName(bin));
>  KStartupInfo::sendChangeXcb(s_startup_notify_connection, 
> s_startup_notify_screen, id, data);
>  xcb_flush(s_startup_notify_connection);
> 
> 
> I'll also try to harden KWindowSystem against that.

pushed that with 
http://commits.kde.org/kinit/e39bd4eb3d39a19543841eda6188118f58382ab6 - thanks 
to Elias and Luca for giving me good backtraces and testing.


- Martin


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


On Jan. 25, 2016, 3:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Jan. 25, 2016, 3:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/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: Review Request 126876: Fix QFileDialog::openUrl() for remote files

2016-01-25 Thread David Faure


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




src/platformtheme/kdeplatformfiledialoghelper.cpp (line 195)


OK then you can remove the "is that a BUG" question here.

I'm still curious as to who is passing a url with a filename here; Qt or 
the app.

In any case the comment is confusing, "Qt returns", this is rather about Qt 
calling this method, not "returning".



src/platformtheme/kdeplatformfiledialoghelper.cpp (line 201)


No need for KFileItem just for that, use entry.isDir().


- David Faure


On Jan. 24, 2016, 9:19 p.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Jan. 24, 2016, 9:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

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

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Martin Gräßlin



> On Jan. 25, 2016, 10:36 p.m., Luca Beltrame wrote:
> > FYI, this breaks Plasma 5 startup completely for me. kinit crashes with
> > 
> > ```gdb
> > #0  0x7ff831e58c20 in QObject::thread() const () at 
> > /usr/lib64/libQt5Core.so.5
> > #1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #4  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:485
> > #5  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:479
> > #6  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (argc=32767, _name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", 
> > cwd=, envc=, envs=, 
> > reset_env=false, tty=0x8e55b0 "", avoid_loops=160, startup_id_str=0x1161 
> > )
> > at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
> > #7  0x008e559d in  ()
> > #8  0x in  ()
> > ```
> > 
> > I had to revert it or kdeinit would crash and prevent me from having a 
> > functional session. ;)
> 
> Andreas Hartmetz wrote:
> Same here.

sorry about that, I'm on it.


- Martin


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


On Jan. 25, 2016, 3:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Jan. 25, 2016, 3:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/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: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Martin Gräßlin



> On Jan. 25, 2016, 10:36 p.m., Luca Beltrame wrote:
> > FYI, this breaks Plasma 5 startup completely for me. kinit crashes with
> > 
> > ```gdb
> > #0  0x7ff831e58c20 in QObject::thread() const () at 
> > /usr/lib64/libQt5Core.so.5
> > #1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #4  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:485
> > #5  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:479
> > #6  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (argc=32767, _name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", 
> > cwd=, envc=, envs=, 
> > reset_env=false, tty=0x8e55b0 "", avoid_loops=160, startup_id_str=0x1161 
> > )
> > at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
> > #7  0x008e559d in  ()
> > #8  0x in  ()
> > ```
> > 
> > I had to revert it or kdeinit would crash and prevent me from having a 
> > functional session. ;)
> 
> Andreas Hartmetz wrote:
> Same here.
> 
> Martin Gräßlin wrote:
> sorry about that, I'm on it.

The problem is that kinit is not a QCoreApplication and it crashes when 
accessing the ::instance(). A possible workaround for kinit is (not yet tested):

diff --git a/src/kdeinit/kinit.cpp b/src/kdeinit/kinit.cpp
index 9919547..8b5c926 100644
--- a/src/kdeinit/kinit.cpp
+++ b/src/kdeinit/kinit.cpp
@@ -375,7 +375,7 @@ static void init_startup_info(KStartupInfoId , const 
QByteArray ,
 X11_startup_notify_fd = 
xcb_get_file_descriptor(s_startup_notify_connection);
 NETRootInfo rootInfo(s_startup_notify_connection, NET::CurrentDesktop);
 KStartupInfoData data;
-data.setDesktop(rootInfo.currentDesktop());
+data.setDesktop(rootInfo.currentDesktop(true));
 data.setBin(QFile::decodeName(bin));
 KStartupInfo::sendChangeXcb(s_startup_notify_connection, 
s_startup_notify_screen, id, data);
 xcb_flush(s_startup_notify_connection);


I'll also try to harden KWindowSystem against that.


- Martin


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


On Jan. 25, 2016, 3:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Jan. 25, 2016, 3:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/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

Review Request 126883: Add Package::cryptographicHash(QCryptographicHash::Algorithm)

2016-01-25 Thread Martin Gräßlin


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

Review request for KDE Frameworks, Plasma and Marco Martin.


Repository: kpackage


Description
---

This method is intended to replace contentsHash which operates only
on Sha1. In order to support more secure hashing algorithmns and also
to support future developments the new implementation does not hard
code the algorithm but allows to specify it. By that the existing
implementation can just delegate to the new one.

Another change in the implementation is that the new cryptographicHash
method returns a QByteArray instead of a QString. As only a hex
representation of the hash is returned the conversion to QString is
not necessary.

Package::contentsHash() is marked as deprecated.


Diffs
-

  autotests/plasmoidpackagetest.cpp 69f45f57bbec5c0f6d2268869249606c50338765 
  src/kpackage/package.h b97b70be42df7d9fd09e3fb4e7b82b00ef970be3 
  src/kpackage/package.cpp 4c0f671965d9ed33b9f52b06205bf6ffcda94c8a 

Diff: https://git.reviewboard.kde.org/r/126883/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: Review Request 126314: Port klauncher to xcb

2016-01-25 Thread Martin Gräßlin


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

(Updated Jan. 25, 2016, 3:34 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit b958641f5f015e2452a3916a9b19e9f5caadc44a by Martin 
Gräßlin to branch master.


Repository: kinit


Description
---

Port klauncher to xcb


Diffs
-

  src/klauncher/CMakeLists.txt 746edfac0b840aa033be219ae5d094689006db6f 
  src/klauncher/klauncher.h e155f72d76d4f99172646ab797ec8c4dd006ddf7 
  src/klauncher/klauncher.cpp 8b3d343b9fb2c852ea2d6c559f13c96a0467d72b 

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: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Martin Gräßlin


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

(Updated Jan. 25, 2016, 3:34 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit 527c3490a867b7a0a22a45a8d5d7a706578f9503 by Martin 
Gräßlin to branch master.


Repository: kinit


Description
---

By changing to xcb we can use NETRootInfo to get the current desktop
and don't need to duplicate the logic. Also it means that more code
is ported from XLib to xcb and might allow us to drop the XLib
dependency in future.


Diffs
-

  CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
  src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
  src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
  src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 

Diff: https://git.reviewboard.kde.org/r/126313/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: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Luca Beltrame


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



FYI, this breaks Plasma 5 startup completely for me. kinit crashes with

```gdb
#0  0x7ff831e58c20 in QObject::thread() const () at 
/usr/lib64/libQt5Core.so.5
#1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
/usr/lib64/libKF5WindowSystem.so.5
#2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
/usr/lib64/libKF5WindowSystem.so.5
#3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
/usr/lib64/libKF5WindowSystem.so.5
#4  0x00408d09 in launch(int, char const*, char const*, char const*, 
int, char const*, bool, char const*, bool, char const*) (this=0x8e4458)
at /usr/include/qt5/QtCore/qbytearray.h:485
#5  0x00408d09 in launch(int, char const*, char const*, char const*, 
int, char const*, bool, char const*, bool, char const*) (this=0x8e4458)
at /usr/include/qt5/QtCore/qbytearray.h:479
#6  0x00408d09 in launch(int, char const*, char const*, char const*, 
int, char const*, bool, char const*, bool, char const*) (argc=32767, 
_name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", cwd=, envc=, envs=, reset_env=false, tty=0x8e55b0 
"", avoid_loops=160, startup_id_str=0x1161 )
at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
#7  0x008e559d in  ()
#8  0x in  ()
```

I had to revert it or kdeinit would crash and prevent me from having a 
functional session. ;)

- Luca Beltrame


On Gen. 25, 2016, 3:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Gen. 25, 2016, 3:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/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: Review Request 126870: Tooltip animation transition in the panel decrease it performance after continous usage

2016-01-25 Thread Martin Gräßlin


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



Pointing out the obvious: though shall not animate window positions! This needs 
to be moved to KWin (I'm disappointed that such code still exists in 
frameworks). Which would kind of solve the problems automatically.

Of course that comment is orthogonal to this review request: improvement is 
always good.


src/declarativeimports/core/tooltip.cpp (line 160)


why bind it to compositingActive?


- Martin Gräßlin


On Jan. 24, 2016, 4:20 p.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126870/
> ---
> 
> (Updated Jan. 24, 2016, 4:20 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Bugs: 354353
> http://bugs.kde.org/show_bug.cgi?id=354353
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Continous usage, read as suspend/resume/use and so on, cause *cool* tooltip 
> animation transition between item to decrease it permormance and looks quite 
> ugly. If add new panel animation is *cool* as is. The perfermonce decrease 
> can be cause due to swap use, or QPropertyAnimation issue, however we (you 
> KDE defs and i *like a user*) wants plasmashell and it's components (like 
> panel) to works *forever*
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/tooltip.cpp a5e223b 
>   src/declarativeimports/core/tooltipdialog.h 2ea8af9 
>   src/declarativeimports/core/tooltipdialog.cpp 6c3712e 
> 
> Diff: https://git.reviewboard.kde.org/r/126870/diff/
> 
> 
> Testing
> ---
> 
> Workaround is quite simple:
> Animation is refreshed as is, pointer hasn't same lifetime as panel
> + Animation is disabled if compositing is not active
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

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

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

2016-01-25 Thread David Faure



> On Jan. 24, 2016, 10:01 p.m., David Faure wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 195
> > 
> >
> > Yes, this definitely looks like it should be fixed in Qt instead (and 
> > this code can be kept with a #ifdef QT_VERSION if you want).
> > 
> > The method is called setDirectory, I expect it to get a directory.
> 
> Kåre Särs wrote:
> OK, but what whould I put as QT_VERSION? ;) The bug is present in at 
> least Qt 5.5.1. I have not tried with the Qt 5.6 branch.
> 
> I wonder if it is actually fixable in Qt. How does Qt determine what is a 
> file and what is a directory? What if you name a directory "foo.doc" and you 
> cannot use QFileInfo::isDir() on it.

Sure, but how do we end up with a file passed to setDirectory? The API (e.g. 
getOpenFileUrls) takes a QUrl dir, surely that's supposed to be a dir. Is that 
where the file comes from? Or is QFileDialog getting confused somewhere? I just 
don't see why we have to determine "is this url a file or a directory" when the 
API is clear about the directory argument. But I assume I'm looking at the 
wrong place, setDirectory is also called with some other URL?
I'm asking for some debugging inside Qt, to find out how we end up with 
setDirectory called with a file URL, and then if it's indeed a bug in Qt fixing 
it there, and only then this will answer the QT_VERSION question ;)


- David


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


On Jan. 24, 2016, 9:19 p.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Jan. 24, 2016, 9:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

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

Re: Making polkit-qt-1 a tier1 framework

2016-01-25 Thread Martin Graesslin

On Saturday, January 23, 2016 11:59:28 AM CET David Faure wrote:
> On Thursday 14 January 2016 13:20:01 Martin Gräßlin wrote:
> > Hi all,
> > 
> > I want to suggest to move polkit-qt-1 [1] from kdesupport to frameworks.
> > Reasons are:
> > 
> > * kdesupport is basically what became tier1 in frameworks
> > * it's used by other frameworks, e.g. KAuth (tier 2) and in
> > kde/workspace
> > * polkit-qt-1 is currently in a not really released state (last release
> > in 2014, quite a few bugfixes around)
> > 
> > By moving it to frameworks we get closer to getting rid of kdesupport
> > and get the making releases problem solved once and for all: bug fixes
> > will get to users in a timely manner.
> > 
> > Opinions?
> 
> Sounds good. A few questions come to mind:
> 
> - Will you be the official maintainer? (Otherwise who will?)

eh no, my domain knowledge is not enough and I already maintain too much.

My suggestion for maintainer is shared responsibility of Plasma team. Not 
optimal, but still better than the unmaintained state it's currently in. And 
Plasma needs polkit.

> 
> - Is this an opportunity for a better name? I keep wondering why this lib
> targets Qt 1 :-)

The name makes kind of sense. It's a Qt wrapper for polkit-1. If you have a 
better name suggestion: sure, but it shouldn't cause breakage for existing 
code.

Cheers
Martin

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126880: Fix QFileDialog::openUrl() for remote files

2016-01-25 Thread David Faure


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



Hmm, see https://git.reviewboard.kde.org/r/126876

- David Faure


On Jan. 24, 2016, 11:24 p.m., Alex Richardson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126880/
> ---
> 
> (Updated Jan. 24, 2016, 11:24 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 345253
> https://bugs.kde.org/show_bug.cgi?id=345253
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> QFileDialog doesn't make an attempt to determine whether remote URLs are
> files or directories so it just passes the file URL to the platform
> file dialog which caused an error popup and a nonexistent directory to
> be selected.
> 
> BUG: 345253
> 
> Supersedes https://git.reviewboard.kde.org/r/126831/
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 
> 11e7efbb66948da40bf19f430f8020f95d0233f8 
> 
> Diff: https://git.reviewboard.kde.org/r/126880/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

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

Re: Review Request 126813: Fix build with older polkit

2016-01-25 Thread Martin Gräßlin


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


Ship it!




Ship It!

- Martin Gräßlin


On Jan. 21, 2016, 5:24 p.m., Alex Richardson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126813/
> ---
> 
> (Updated Jan. 21, 2016, 5:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> Seems like the function exists, but the header declaration is missing
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
>   core/polkitqt1-subject.h 03028f636d7efc154138821419a704b661a7aeac 
>   core/polkitqt1-subject.cpp ecb4c0e216d5bacf5dff5cf100611b941d3e8fbd 
>   polkitqt1-config.h.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126813/diff/
> 
> 
> Testing
> ---
> 
> compiles now
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

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

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

2016-01-25 Thread Kåre Särs



> On Jan. 24, 2016, 10:01 p.m., David Faure wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 195
> > 
> >
> > Yes, this definitely looks like it should be fixed in Qt instead (and 
> > this code can be kept with a #ifdef QT_VERSION if you want).
> > 
> > The method is called setDirectory, I expect it to get a directory.
> 
> Kåre Särs wrote:
> OK, but what whould I put as QT_VERSION? ;) The bug is present in at 
> least Qt 5.5.1. I have not tried with the Qt 5.6 branch.
> 
> I wonder if it is actually fixable in Qt. How does Qt determine what is a 
> file and what is a directory? What if you name a directory "foo.doc" and you 
> cannot use QFileInfo::isDir() on it.
> 
> David Faure wrote:
> Sure, but how do we end up with a file passed to setDirectory? The API 
> (e.g. getOpenFileUrls) takes a QUrl dir, surely that's supposed to be a dir. 
> Is that where the file comes from? Or is QFileDialog getting confused 
> somewhere? I just don't see why we have to determine "is this url a file or a 
> directory" when the API is clear about the directory argument. But I assume 
> I'm looking at the wrong place, setDirectory is also called with some other 
> URL?
> I'm asking for some debugging inside Qt, to find out how we end up with 
> setDirectory called with a file URL, and then if it's indeed a bug in Qt 
> fixing it there, and only then this will answer the QT_VERSION question ;)

Now that I read the documentation again I don't see any mention of it, but if 
you provide a local file url to getOpenFileUrls()'s dir parameter it will open 
the directory of the file and pre-select the file. While this works for local 
files it does not work for remote protocols.

I looked into this last night for a couple of hours, but I have not dug deep 
enough to find it. I do suspect that this maybe hidden feature just can not 
work for remote protocols and that the "fix" has to be in the integration.


- Kåre


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


On Jan. 24, 2016, 9:19 p.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Jan. 24, 2016, 9:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

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

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

2016-01-25 Thread Alex Richardson



> On Jan. 24, 2016, 10:01 p.m., David Faure wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 195
> > 
> >
> > Yes, this definitely looks like it should be fixed in Qt instead (and 
> > this code can be kept with a #ifdef QT_VERSION if you want).
> > 
> > The method is called setDirectory, I expect it to get a directory.
> 
> Kåre Särs wrote:
> OK, but what whould I put as QT_VERSION? ;) The bug is present in at 
> least Qt 5.5.1. I have not tried with the Qt 5.6 branch.
> 
> I wonder if it is actually fixable in Qt. How does Qt determine what is a 
> file and what is a directory? What if you name a directory "foo.doc" and you 
> cannot use QFileInfo::isDir() on it.
> 
> David Faure wrote:
> Sure, but how do we end up with a file passed to setDirectory? The API 
> (e.g. getOpenFileUrls) takes a QUrl dir, surely that's supposed to be a dir. 
> Is that where the file comes from? Or is QFileDialog getting confused 
> somewhere? I just don't see why we have to determine "is this url a file or a 
> directory" when the API is clear about the directory argument. But I assume 
> I'm looking at the wrong place, setDirectory is also called with some other 
> URL?
> I'm asking for some debugging inside Qt, to find out how we end up with 
> setDirectory called with a file URL, and then if it's indeed a bug in Qt 
> fixing it there, and only then this will answer the QT_VERSION question ;)
> 
> Kåre Särs wrote:
> Now that I read the documentation again I don't see any mention of it, 
> but if you provide a local file url to getOpenFileUrls()'s dir parameter it 
> will open the directory of the file and pre-select the file. While this works 
> for local files it does not work for remote protocols.
> 
> I looked into this last night for a couple of hours, but I have not dug 
> deep enough to find it. I do suspect that this maybe hidden feature just can 
> not work for remote protocols and that the "fix" has to be in the integration.

The docs say `The file dialog's working directory will be set to dir. If dir 
includes a file name, the file will be selected.` So I think this should also 
work for remote files.


- Alex


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


On Jan. 24, 2016, 9:19 p.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Jan. 24, 2016, 9:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

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

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

2016-01-25 Thread Andreas Hartmetz



> On Jan. 25, 2016, 9:36 p.m., Luca Beltrame wrote:
> > FYI, this breaks Plasma 5 startup completely for me. kinit crashes with
> > 
> > ```gdb
> > #0  0x7ff831e58c20 in QObject::thread() const () at 
> > /usr/lib64/libQt5Core.so.5
> > #1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #4  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:485
> > #5  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:479
> > #6  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (argc=32767, _name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", 
> > cwd=, envc=, envs=, 
> > reset_env=false, tty=0x8e55b0 "", avoid_loops=160, startup_id_str=0x1161 
> > )
> > at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
> > #7  0x008e559d in  ()
> > #8  0x in  ()
> > ```
> > 
> > I had to revert it or kdeinit would crash and prevent me from having a 
> > functional session. ;)

Same here.


- Andreas


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


On Jan. 25, 2016, 2:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Jan. 25, 2016, 2:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/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: Review Request 126313: Use an xcb for interaction with KStartupInfo

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

Review Request 126883: Add Package::cryptographicHash(QCryptographicHash::Algorithm)

Re: Review Request 126314: Port klauncher to xcb

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

Re: Review Request 126870: Tooltip animation transition in the panel decrease it performance after continous usage

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

Re: Making polkit-qt-1 a tier1 framework

Re: Review Request 126880: Fix QFileDialog::openUrl() for remote files

Re: Review Request 126813: Fix build with older polkit

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

16 matches

Site Navigation

Mail list logo

Footer information