[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 113971: improve screenlocker multiscreen handling
From: Thomas_Lübking <thomas.luebking () gmail ! com>
Date: 2013-11-20 17:01:21
Message-ID: 20131120170121.585.82857 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113971/#review44054
-----------------------------------------------------------
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31580>
this is just for me, since i do not intend to install a "screensaver".
It shows a white window instead when set 1
Can be dropped from final patch if there's no interest from other ppl.
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31581>
no idea why this was still there (probably my fault) but we filter the \
application, ie. get events for every widget and there was no smart filter juggling \
as well.
-> removed because it just pollutes event forwarding (since we receive every \
keyboard event on this twice)
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31582>
this loop is required to make the qml/graphicsscene properly hadnle the forwarded \
events.
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31583>
the rest here searches for the window under the mouse, assumes that is where the \
focus shall go and passes it there.
If the assumption is wrong, the only "problem" is that the lineedit is not \
focused (but due to event sharing you should see the input nevertheless)
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31585>
I twisted this check.
The reason is that if you juggle the mouse, only the screensaver below will hide.
So far the result was that the next keypress would hide the other screensavers \
and only the second next and following would be actual password input.
Now the remaining screensavers are just kept intact and you operate on the only \
visible item.
I assume that users will move the mouse until they see the greeter (if the first \
hidden screensaver was on a powered off screen) so this should be no issue (while the \
eaten second key kinda was to me)
ksmserver/screenlocker/greeter/greeterapp.cpp
<http://git.reviewboard.kde.org/r/113971/#comment31586>
This is obviously very important.
If we enter a recursion here, the user will not be able to enter a password on a \
multiscreen context
- Thomas Lübking
On Nov. 20, 2013, 4:52 p.m., Thomas Lübking wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113971/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2013, 4:52 p.m.)
>
>
> Review request for Plasma, Christoph Feck and Martin Gräßlin.
>
>
> Bugs: 311188
> http://bugs.kde.org/show_bug.cgi?id=311188
>
>
> Repository: kde-workspace
>
>
> Description
> -------
>
> - pass focus to greeter under the mouse
> - share keyboard events
> - let through keyboard events for one hidden locker
> (mouse wakeup case)
> - remove double filtering
>
> @Christoph:
> I meant a real human. A shared responsibility simply means that no one is \
> responsible.
> @Martin:
> I found the dupe and it's assigned to you. Do you feel in charge here or do you \
> know whom to addd?
>
> Diffs
> -----
>
> ksmserver/screenlocker/greeter/greeterapp.h 951b1e3
> ksmserver/screenlocker/greeter/greeterapp.cpp bf33945
>
> Diff: http://git.reviewboard.kde.org/r/113971/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Thomas Lübking
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113971/">http://git.reviewboard.kde.org/r/113971/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line56" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">along \
with this program. If not, see <http://www.gnu.org/licenses/>.</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">56</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="cp">#define TEST_SCREENSAVER 0</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this is \
just for me, since i do not intend to install a "screensaver". It shows a \
white window instead when set 1
Can be dropped from final patch if there's no interest from other ppl.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line155" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
UnlockApp::desktopResized()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">148</font></th> <td bgcolor="#ffc5ce" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">view</span><span class="o">-></span><span \
class="n">installEventFilter</span><span class="p">(</span><span \
class="k">this</span><span class="p">);</span></pre></td> <th bgcolor="#ebb1ba" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#ffc5ce" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">no idea why \
this was still there (probably my fault) but we filter the application, ie. get \
events for every widget and there was no smart filter juggling as well.
-> removed because it just pollutes event forwarding (since we receive every \
keyboard event on this twice)</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line238" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
UnlockApp::desktopResized()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">237</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">foreach</span> <span class="p">(</span><span \
class="n">QDeclarativeView</span> <span class="o">*</span><span \
class="n">view</span><span class="p">,</span> <span class="n">m_views</span><span \
class="p">)</span> <span class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this loop \
is required to make the qml/graphicsscene properly hadnle the forwarded events.</pre> \
</div> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line243" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
UnlockApp::desktopResized()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">242</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">foreach</span> <span class="p">(</span><span \
class="n">QDeclarativeView</span> <span class="o">*</span><span \
class="n">view</span><span class="p">,</span> <span class="n">m_views</span><span \
class="p">)</span> <span class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the rest \
here searches for the window under the mouse, assumes that is where the focus shall \
go and passes it there.
If the assumption is wrong, the only "problem" is that the lineedit is not \
focused (but due to event sharing you should see the input nevertheless)</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line349" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool \
UnlockApp::eventFilter(QObject *obj, QEvent *event)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">313</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="kt">bool</span> <span class="n">saverVisible</span> <span class="o">=</span> \
<span class="nb"><span class="hl">fals</span>e</span><span \
class="p">;</span></pre></td> <th bgcolor="#e9eaa8" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">348</font></th> <td bgcolor="#fdfebc" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span \
class="n">saverVisible</span> <span class="o">=</span> <span class="nb"><span \
class="hl">tru</span>e</span><span class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I twisted \
this check. The reason is that if you juggle the mouse, only the screensaver below \
will hide.
So far the result was that the next keypress would hide the other screensavers and \
only the second next and following would be actual password input.
Now the remaining screensavers are just kept intact and you operate on the only \
visible item.
I assume that users will move the mouse until they see the greeter (if the first \
hidden screensaver was on a powered off screen) so this should be no issue (while the \
eaten second key kinda was to me)</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/113971/diff/1/?file=215038#file215038line428" \
style="color: black; font-weight: bold; text-decoration: \
underline;">ksmserver/screenlocker/greeter/greeterapp.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">427</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">removeEventFilter</span><span class="p">(</span><span \
class="k">this</span><span class="p">);</span> <span class="c1">// prevent \
recursion</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is \
obviously very important. If we enter a recursion here, the user will not be able to \
enter a password on a multiscreen context</pre> </div>
<br />
<p>- Thomas Lübking</p>
<br />
<p>On November 20th, 2013, 4:52 p.m. UTC, Thomas Lübking wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Plasma, Christoph Feck and Martin Gräßlin.</div>
<div>By Thomas Lübking.</div>
<p style="color: grey;"><i>Updated Nov. 20, 2013, 4:52 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=311188">311188</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</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;">- pass focus to greeter under the mouse
- share keyboard events
- let through keyboard events for one hidden locker
(mouse wakeup case)
- remove double filtering
@Christoph:
I meant a real human. A shared responsibility simply means that no one is \
responsible.
@Martin:
I found the dupe and it's assigned to you. Do you feel in charge here or do you \
know whom to addd?</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>ksmserver/screenlocker/greeter/greeterapp.h <span style="color: \
grey">(951b1e3)</span></li>
<li>ksmserver/screenlocker/greeter/greeterapp.cpp <span style="color: \
grey">(bf33945)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113971/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic