Re: Review Request 121238: Add support for more multimedia keys

[Albert Astals Cid]

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

(Updated Nov. 25, 2014, 12:13 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Martin Gräßlin.


Repository: kwindowsystem


Description
---

See summary


Diffs
-

  src/kkeyserver_x11.cpp 22ce5e2 

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


Testing
---

Now i can use my yet-to-be-published ktouchpadenabler using KF5 instead of 
plain X.


Thanks,

Albert Astals Cid

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

Re: Review Request 121238: Add support for more multimedia keys

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Nov. 25, 2014, 12:04 a.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121238/
> ---
> 
> (Updated Nov. 25, 2014, 12:04 a.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> See summary
> 
> 
> Diffs
> -
> 
>   src/kkeyserver_x11.cpp 22ce5e2 
> 
> Diff: https://git.reviewboard.kde.org/r/121238/diff/
> 
> 
> Testing
> ---
> 
> Now i can use my yet-to-be-published ktouchpadenabler using KF5 instead of 
> plain X.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Review Request 121238: Add support for more multimedia keys

[Albert Astals Cid]

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

Review request for KDE Frameworks and Martin Gräßlin.


Repository: kwindowsystem


Description
---

See summary


Diffs
-

  src/kkeyserver_x11.cpp 22ce5e2 

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


Testing
---

Now i can use my yet-to-be-published ktouchpadenabler using KF5 instead of 
plain X.


Thanks,

Albert Astals Cid

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

Re: Review Request 121233: Do not drop ASN passed to KRun when executing desktop files

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 7:53 p.m.)


Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

KDesktopFileActions::run did not accept the optional argument
QByteArray asn which meant that any application startup id passed
to KRun got dropped at this point when launching a desktop file.

This change ensures that an ASN is passed along. This way we can
properly pass around the application which launched the process
and the timestamp which triggered the launch (otherwise klauncher
needs a roundtrip to the X-Server to fetch the latest timestamp).


Diffs
-

  src/widgets/kdesktopfileactions.cpp 0cbeb00f0c0344b5b204f0d18b7c269da6c99e7f 
  src/widgets/kdesktopfileactions.h 7b96ff079c752436aebd0b134b930171ea088f48 
  src/widgets/krun.cpp c623b583716e492622e0d1d1c9cec485c6bcf163 

Diff: https://git.reviewboard.kde.org/r/121233/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 121233: Do not drop ASN passed to KRun when executing desktop files

[Martin Gräßlin]

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

Review request for KDE Frameworks.


Repository: kio


Description
---

KDesktopFileActions::run did not accept the optional argument
QByteArray asn which meant that any application startup id passed
to KRun got dropped at this point when launching a desktop file.

This change ensures that an ASN is passed along. This way we can
properly pass around the application which launched the process
and the timestamp which triggered the launch (otherwise klauncher
needs a roundtrip to the X-Server to fetch the latest timestamp).


Diffs
-

  src/widgets/kdesktopfileactions.cpp 0cbeb00f0c0344b5b204f0d18b7c269da6c99e7f 
  src/widgets/kdesktopfileactions.h 7b96ff079c752436aebd0b134b930171ea088f48 
  src/widgets/krun.cpp c623b583716e492622e0d1d1c9cec485c6bcf163 

Diff: https://git.reviewboard.kde.org/r/121233/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

Jenkins build became unstable: kservice_master_qt5 #204

[KDE CI System]

See 

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

Re: Review Request 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Albert Astals Cid]

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


FWIW i've tried this patch and it fixes the firefox problem for me :)

- Albert Astals Cid


On nov. 24, 2014, 3:06 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated nov. 24, 2014, 3:06 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121230: Fix passing of DESKTOP_STARTUP_ID to child process in kioexec

[Martin Gräßlin]

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

Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

The code was hard disabled due to usage of Q_WS_X11. The task is
to get the startupid passed to kioexec and forward it to the
process started by kioexec. As Qt's xcb plugin removes the
DESKTOP_STARTUP_ID environment variable once it is read, this needs
special handling to setup the enivornment for the process to be
started.

Unfortunately the fix requires Qt 5.4 as the startupId is only
provided by QX11Extras since 5.4.


Diffs
-

  CMakeLists.txt 888edff2bee476b4d2098b375a38a2b3a86135e8 
  src/kioexec/CMakeLists.txt 6c587e19310a5395f0db455f48fe18a27e9ec386 
  src/kioexec/config-kioexec.h.cmake PRE-CREATION 
  src/kioexec/main.cpp f0e282394c4f65df0bf52141290d37ba6b32ad8c 

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


Testing
---

used
DESKTOP_STARTUP_ID="FOO" kioexec "gwenview %U" file:///path/to/local/file.png

with and without the change and inspected /proc/x/environ for the started 
gwenview process. Without the patch DESKTOP_STARTUP_ID is not set, with the 
patch it's properly set to "FOO" in this case.


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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Thomas Lübking]

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

Ship it!


Ship It!

- Thomas Lübking


On Nov. 24, 2014, 3:06 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 3:06 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Thomas Lübking]

> On Nov. 24, 2014, 2:01 nachm., Martin Gräßlin wrote:
> > after checking 
> > https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ : 
> > this is a BC compatible change, but not SC. Seems like one needs to use a 
> > different method name :-(
> 
> Thomas Lübking wrote:
> Where did you read this?
> void foo() -> void foo(int bar = 1);
> isn't BC, but it is oc SC.
> 
> What you need to do is to overload explicitly:
> void foo() -> void foo(); void foo(int bar); // no default!
> 
> Thomas Lübking wrote:
> > The solution in this change is to get the current timestamp from the X 
> server in case the appUserTime is 0.
> 
> This is (luckily) not what the patch does atm. (I took the default w/ a 
> grain of salt, as the proposed behavior would have prevented to explicitly 
> start the application w/o taking the focus)
> 
> Martin Gräßlin wrote:
> yoy cannot add an overload (BC, but not SC: makes &func ambiguous), 
> adding overloads to already overloaded functions is ok (any use of &func 
> already needed a cast).
> 
> to your other comment: rbt failed updating the comment.

Ah - fucktors, e... "functors" ;-)
Yes, indeed :-(


- Thomas


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


On Nov. 24, 2014, 3:06 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 3:06 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Martin Gräßlin]

> On Nov. 24, 2014, 3:01 p.m., Martin Gräßlin wrote:
> > after checking 
> > https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ : 
> > this is a BC compatible change, but not SC. Seems like one needs to use a 
> > different method name :-(
> 
> Thomas Lübking wrote:
> Where did you read this?
> void foo() -> void foo(int bar = 1);
> isn't BC, but it is oc SC.
> 
> What you need to do is to overload explicitly:
> void foo() -> void foo(); void foo(int bar); // no default!
> 
> Thomas Lübking wrote:
> > The solution in this change is to get the current timestamp from the X 
> server in case the appUserTime is 0.
> 
> This is (luckily) not what the patch does atm. (I took the default w/ a 
> grain of salt, as the proposed behavior would have prevented to explicitly 
> start the application w/o taking the focus)

yoy cannot add an overload (BC, but not SC: makes &func ambiguous), adding 
overloads to already overloaded functions is ok (any use of &func already 
needed a cast).

to your other comment: rbt failed updating the comment.


- Martin


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


On Nov. 24, 2014, 4:06 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 4:06 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Thomas Lübking]

> On Nov. 24, 2014, 2:01 nachm., Martin Gräßlin wrote:
> > after checking 
> > https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ : 
> > this is a BC compatible change, but not SC. Seems like one needs to use a 
> > different method name :-(
> 
> Thomas Lübking wrote:
> Where did you read this?
> void foo() -> void foo(int bar = 1);
> isn't BC, but it is oc SC.
> 
> What you need to do is to overload explicitly:
> void foo() -> void foo(); void foo(int bar); // no default!

> The solution in this change is to get the current timestamp from the X server 
> in case the appUserTime is 0.

This is (luckily) not what the patch does atm. (I took the default w/ a grain 
of salt, as the proposed behavior would have prevented to explicitly start the 
application w/o taking the focus)


- Thomas


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


On Nov. 24, 2014, 3:06 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 3:06 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Thomas Lübking]

> On Nov. 24, 2014, 2:01 nachm., Martin Gräßlin wrote:
> > after checking 
> > https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ : 
> > this is a BC compatible change, but not SC. Seems like one needs to use a 
> > different method name :-(

Where did you read this?
void foo() -> void foo(int bar = 1);
isn't BC, but it is oc SC.

What you need to do is to overload explicitly:
void foo() -> void foo(); void foo(int bar); // no default!


- Thomas


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


On Nov. 24, 2014, 3:06 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 3:06 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 4:06 p.m.)


Review request for KDE Frameworks.


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


Repository: kwindowsystem


Description
---

KStartupInfo::createNewStartupId is supposed to return a new
id with a properly setup user timestamp. But if QX11Info::appUserTime
returns 0 this was included in the id and cannot be considered a
properly setup user timestamp. An app user time of 0 is the case
for klauncher. If an application uses this timestamp of 0 passed from
the startup notification it causes problems with passing focus to it
from KWin. The application sets the timestamp in the
_NET_WM_USER_TIME property and the value "0" has the special meaning
of requesting to not be initially mapped. KWin honors that and doesn't
focus the window. An application is not supposed to interpret a 0 time
stamp passed through the startup notification as a special value to get
the current time. It is totally fine to expect that it gets a proper
value passed. Thus one cannot blame applications for not interpreting
this value accordingly. An example for an application passing the 0
to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
session through e.g. the launcher results in KWin not passing focus
to it and instead demands attention. This is clearly wrong and not
intended.

The solution in this change is to get the current timestamp from
the X server in case the appUserTime is 0. This fixes the described
problem with Firefox: it now gets properly focused on starting
through a launcher.


Diffs (updated)
-

  autotests/kstartupinfo_unittest.cpp f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
  src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
  src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 

Diff: https://git.reviewboard.kde.org/r/121197/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 121172: Move libgit2-related macro defs into config.h

[Kevin Funk]

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

(Updated Nov. 24, 2014, 2:31 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and Christoph Cullmann.


Repository: ktexteditor


Description
---

Only recompile dependent files in case of dep changes


Diffs
-

  CMakeLists.txt 5a597818e8b5a16899c10fd4a460e09bb233 
  config.h.cmake e1c9f34cf24eb516c540db3288c93ad5bd093df3 
  src/document/katedocument.cpp aac17c719cbabf78c2135062fe566671713cea02 
  src/utils/kateglobal.cpp 473240514eaeb7f108a5bc0a06e182d4aacc7a80 

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


Testing
---


Thanks,

Kevin Funk

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

Re: Review Request 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Martin Gräßlin]

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


after checking 
https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ : this 
is a BC compatible change, but not SC. Seems like one needs to use a different 
method name :-(

- Martin Gräßlin


On Nov. 24, 2014, 2:38 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121197/
> ---
> 
> (Updated Nov. 24, 2014, 2:38 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 337798
> https://bugs.kde.org/show_bug.cgi?id=337798
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KStartupInfo::createNewStartupId is supposed to return a new
> id with a properly setup user timestamp. But if QX11Info::appUserTime
> returns 0 this was included in the id and cannot be considered a
> properly setup user timestamp. An app user time of 0 is the case
> for klauncher. If an application uses this timestamp of 0 passed from
> the startup notification it causes problems with passing focus to it
> from KWin. The application sets the timestamp in the
> _NET_WM_USER_TIME property and the value "0" has the special meaning
> of requesting to not be initially mapped. KWin honors that and doesn't
> focus the window. An application is not supposed to interpret a 0 time
> stamp passed through the startup notification as a special value to get
> the current time. It is totally fine to expect that it gets a proper
> value passed. Thus one cannot blame applications for not interpreting
> this value accordingly. An example for an application passing the 0
> to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
> session through e.g. the launcher results in KWin not passing focus
> to it and instead demands attention. This is clearly wrong and not
> intended.
> 
> The solution in this change is to get the current timestamp from
> the X server in case the appUserTime is 0. This fixes the described
> problem with Firefox: it now gets properly focused on starting
> through a launcher.
> 
> 
> Diffs
> -
> 
>   autotests/kstartupinfo_unittest.cpp 
> f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
>   src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
>   src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 
> 
> Diff: https://git.reviewboard.kde.org/r/121197/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 2:38 nachm.)


Review request for KDE Frameworks.


Changes
---

createNewStartupId now takes an optional timestamp argument. If not provided it 
uses getTimestamp. Thus all existing code is no longer relying on the (broken) 
appUserTime.


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


Repository: kwindowsystem


Description
---

KStartupInfo::createNewStartupId is supposed to return a new
id with a properly setup user timestamp. But if QX11Info::appUserTime
returns 0 this was included in the id and cannot be considered a
properly setup user timestamp. An app user time of 0 is the case
for klauncher. If an application uses this timestamp of 0 passed from
the startup notification it causes problems with passing focus to it
from KWin. The application sets the timestamp in the
_NET_WM_USER_TIME property and the value "0" has the special meaning
of requesting to not be initially mapped. KWin honors that and doesn't
focus the window. An application is not supposed to interpret a 0 time
stamp passed through the startup notification as a special value to get
the current time. It is totally fine to expect that it gets a proper
value passed. Thus one cannot blame applications for not interpreting
this value accordingly. An example for an application passing the 0
to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
session through e.g. the launcher results in KWin not passing focus
to it and instead demands attention. This is clearly wrong and not
intended.

The solution in this change is to get the current timestamp from
the X server in case the appUserTime is 0. This fixes the described
problem with Firefox: it now gets properly focused on starting
through a launcher.


Diffs (updated)
-

  autotests/kstartupinfo_unittest.cpp f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
  src/kstartupinfo.h ff4454b4102ecaea7067adb8c3a56d7f500250e0 
  src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 

Diff: https://git.reviewboard.kde.org/r/121197/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 120257: Add support for initial mapping state of WM_HINTS

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 9:35 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and kwin.


Repository: kwindowsystem


Description
---

To complement the other flags already taken from WM_HINTS, we also
read the initial mapping state.


Diffs
-

  autotests/netwininfotestclient.cpp 2795cbf11277d6dc78d24979797e6d6c9cd3750e 
  src/netwm.h 0535c39efc3d4fc428545abd599ce927fbf8903d 
  src/netwm.cpp 3107a2388c2fb3f1a8ad97e466be40e6977e064b 
  src/netwm_def.h c8b1ba0a99a5f1e8a4939d0304b0facde25c59a8 
  src/netwm_p.h 3bb8213acd39f86c8c1c49015243d2bd3e82a8ba 

Diff: https://git.reviewboard.kde.org/r/120257/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: Re: pkexec vs kdesu

[Martin Gräßlin]

On Sunday 23 November 2014 17:14:02 Harald Sitter wrote:
> On Sun, Nov 23, 2014 at 4:50 PM, David Edmundson
> 
>  wrote:
> > You will probably get massively different behaviour regarding your env.
> > 
> > pkexec won't copy anything, sudo will.
> > Without $DISPLAY graphical apps are out.
> 
> Surely not an unsolvable problem. Clearly wayland fixes this ;)

even if it's meant as a joke, I must point out that the opposite is the case 
here: it will horribly fail on Wayland if it assumes a $DISPLAY!

Also I must point out that there are horrible interaction problems on the X11 
platform. KWin does not get enough information to prevent focus stealing 
prevention from kicking in and thus the dialogs do not get focus. This problem 
(to my knowledge) does not appear with kdesu. Thus I would consider switching 
all kdesu to polkit as a regression. This is a long standing issue which must 
be fixed by polkit and hasn't been addressed in years although the problem and 
how to solve had been explained by the current and previous KWin maintainer.

This problem is not unique to polkit, to be fair. We see the same problem with 
akonadi asking for passwords and also with the GPG ask dialog.

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 120917: Fix memory leak for NETWinInfoPrivate::client_machine

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 9:28 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kwin, Bhushan Shah, and Lukáš Tinkl.


Repository: kwindowsystem


Description
---

Got a valgrind report for a possible leak in KWindowSystem and could
confirm it with valgrind on the auto tests. The auto tests are
extended to create "dangerous" situations to expose similar leaks.


Diffs
-

  src/netwm.cpp 7b398d46e9da2c85a96ab67b6c0f97b22ceae4a3 
  autotests/netwininfotestclient.cpp c6905a6cc892096466c3e8b1411e7d3049c72dfa 
  autotests/netwininfotestwm.cpp 7704b243b3055090ed1037cdd5f34305dd0ae401 

Diff: https://git.reviewboard.kde.org/r/120917/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 121197: KStartupInfo: use QX11Info::getTimestamp if appUserTime is 0

[Martin Gräßlin]

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

(Updated Nov. 24, 2014, 9:09 a.m.)


Review request for KDE Frameworks, kwin and Plasma.


Changes
---

added bug


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


Repository: kwindowsystem


Description (updated)
---

KStartupInfo::createNewStartupId is supposed to return a new
id with a properly setup user timestamp. But if QX11Info::appUserTime
returns 0 this was included in the id and cannot be considered a
properly setup user timestamp. An app user time of 0 is the case
for klauncher. If an application uses this timestamp of 0 passed from
the startup notification it causes problems with passing focus to it
from KWin. The application sets the timestamp in the
_NET_WM_USER_TIME property and the value "0" has the special meaning
of requesting to not be initially mapped. KWin honors that and doesn't
focus the window. An application is not supposed to interpret a 0 time
stamp passed through the startup notification as a special value to get
the current time. It is totally fine to expect that it gets a proper
value passed. Thus one cannot blame applications for not interpreting
this value accordingly. An example for an application passing the 0
to the _NET_WM_USER_TIME is Firefox. Starting Firefox in a Plasma 5
session through e.g. the launcher results in KWin not passing focus
to it and instead demands attention. This is clearly wrong and not
intended.

The solution in this change is to get the current timestamp from
the X server in case the appUserTime is 0. This fixes the described
problem with Firefox: it now gets properly focused on starting
through a launcher.


Diffs
-

  autotests/kstartupinfo_unittest.cpp f4b607d2a21da7dcf32434c00b2bdeeaf401ee65 
  src/kstartupinfo.cpp 03418fce1a154ea388d0f65665dc0c9724a5623a 

Diff: https://git.reviewboard.kde.org/r/121197/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