<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/102671/">http://git.reviewboard.kde.org/r/102671/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 24th, 2011, 12:57 p.m., <b>Olivier Goffart</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/102671/diff/2/?file=36541#file36541line551" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/dialogs/kdialog.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">static QRect screenRect( QWidget *widget, int screen )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">551</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">desktop</span><span class="o">-></span><span class="n"><span class="hl">g</span>eometry</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">551</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">desktop</span><span class="o">-></span><span class="n"><span class="hl">screenG</span>eometry</span><span class="p">(</span><span class="n"><span class="hl">widget</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Are you sure it should not be availableGeometry(widget) so it is consistant with the other case? (or maybe the other case is wrong as well, wr really want to center on the center of the screen, not its available geometry).</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hm. Well if we want to be equivalent to the old behaviour, screenGeometry(widget) is the way to go.
Looking at the case "desktop->isVirtualDesktop() && cg.readEntry( "XineramaEnabled", true ) && cg.readEntry( "XineramaPlacementEnabled", true )" it does make more sense to use availableGeometry(widget) instead
of geometry() of screenGeometry(widget). I can imagine that the difference went unnoticed so far because for 99% of all users the difference lies within ~30 pixels due to the default panel setup.
I'll change the diff to use availableGeometry, we can always choose to use the other version if there are any valid objections to this approach.</pre>
<br />
<p>- Thomas</p>
<br />
<p>On September 23rd, 2011, 12:23 p.m., Thomas Gahr wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Thomas Gahr.</div>
<p style="color: grey;"><i>Updated Sept. 23, 2011, 12:23 p.m.</i></p>
<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;">This patch changes KDialog::screenRect to call QDesktopWidget::screenGeometry(widget) instead of QDesktopWidget::geometry(), which returns the wrong geometry if a second monitor has been connected and disconnected.
This fixes a bug described here:
http://article.gmane.org/gmane.comp.kde.devel.core/71875
The testcase and results can be found here:
http://article.gmane.org/gmane.comp.kde.devel.core/71911
Behaviour of QDesktopWidget::geometry() vs. QDesktopWidget::screenGeometry(widget) are equivalent in case of non-faulty ::geometry() and fix the problem in case of faulty ::geometry(). So I wouldn't consider this a workaround that needs to be maintained/changed back once (if) the bug in Qt has been fixed. On the contrary: using ::screenGeometry(widget) is more verbose than ::geometry(), which make more sense in the context of KDialog::screenRect(QWidget* widget,int screen) IMHO</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;">Tested successfully with 4.7.0 on Fedora 15 and current master... and on branch KDE/4.7 - as expected everything works fine</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>kdeui/dialogs/kdialog.cpp <span style="color: grey">(0cabb85)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102671/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>