'Re: Review Request 121234: [Kickoff] Use latest X11 user time for creating StartupInfoId'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 121234: [Kickoff] Use latest X11 user time for creating StartupInfoId
From:       Sebastian_Kügler <sebas () kde ! org>
Date:       2014-11-25 14:00:05
Message-ID: 20141125140005.11306.33914 () probe ! kde ! org
[Download RAW message or body]

--===============3595414146953704326==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

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

Ship it!

Ship It!

- Sebastian Kügler

On Nov. 24, 2014, 7:05 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121234/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2014, 7:05 p.m.)
> 
> 
> Review request for Plasma, Eike Hein and Vishesh Handa.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> The StartupInfoId is supposed to contain information about the
> process and event which triggered the startup of the application.
> E.g. in the case of Kickoff it should be the last input event and
> the pid of Plasma. By creating the StartupInfoId directly in Kickoff
> and passing it to KRun we can ensure that this information is
> correct.
> 
> The code so far lost this information and the launch information
> was not correct. The timestamp is set to a "random" timestamp after
> the event and the pid is set to the one of the klauncher process.
> Furthermore this saves a roundtrip to the X Server from klauncher.
> 
> 
> @Eike and Vishesh: I'd suggest to add the same change to Kicker and KRunner.
> 
> 
> Diffs
> -----
> 
> applets/kickoff/CMakeLists.txt 817c7e41f4454aab66db5ad84e4b494395e5484f 
> applets/kickoff/core/urlitemlauncher.cpp 2ed074709c88ed10cdd72bae7d2b6bf2879183ae 
> 
> Diff: https://git.reviewboard.kde.org/r/121234/diff/
> 
> 
> Testing
> -------
> 
> started applications through plasmoidviewer and inspected the /proc/x/environ to \
> check the DESKTOP_STARTUP_ID env variable. 
> 
> Thanks,
> 
> Martin Gräßlin
> 
> 

--===============3595414146953704326==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">  \
<tr>  <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/121234/">https://git.reviewboard.kde.org/r/121234/</a>
  </td>
    </tr>
   </table>
   <br />

 <p>Ship it!</p>

 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ship \
It!</p></pre>  <br />

<p>- Sebastian Kügler</p>

<br />
<p>On November 24th, 2014, 7:05 p.m. UTC, Martin Gräßlin wrote:</p>

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;">  <tr>
  <td>

<div>Review request for Plasma, Eike Hein and Vishesh Handa.</div>
<div>By Martin Gräßlin.</div>

<p style="color: grey;"><i>Updated Nov. 24, 2014, 7:05 p.m.</i></p>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The StartupInfoId is supposed to contain information \
about the process and event which triggered the startup of the application.
E.g. in the case of Kickoff it should be the last input event and
the pid of Plasma. By creating the StartupInfoId directly in Kickoff
and passing it to KRun we can ensure that this information is
correct.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The code so far lost this information and the launch \
information was not correct. The timestamp is set to a "random" timestamp after
the event and the pid is set to the one of the klauncher process.
Furthermore this saves a roundtrip to the X Server from klauncher.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">@Eike and Vishesh: I'd suggest to add the same change \
to Kicker and KRunner.</p></pre>  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">started applications through plasmoidviewer and \
inspected the /proc/x/environ to check the DESKTOP_STARTUP_ID env variable.</p></pre> \
</td>  </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>applets/kickoff/CMakeLists.txt <span style="color: \
grey">(817c7e41f4454aab66db5ad84e4b494395e5484f)</span></li>

 <li>applets/kickoff/core/urlitemlauncher.cpp <span style="color: \
grey">(2ed074709c88ed10cdd72bae7d2b6bf2879183ae)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/121234/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

--===============3595414146953704326==--

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[prev in list] [next in list] [prev in thread] [next in thread] 