Review Request: KDIalog::screenRect: workaround for faulty QDesktopWidget::geometry

Thomas Gahr identity.kde.org at dadommas.de
Sat Sep 24 14:57:05 BST 2011



> On Sept. 24, 2011, 12:57 p.m., Olivier Goffart wrote:
> > kdeui/dialogs/kdialog.cpp, line 551
> > <http://git.reviewboard.kde.org/r/102671/diff/2/?file=36541#file36541line551>
> >
> >     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).

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.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102671/#review6772
-----------------------------------------------------------


On Sept. 23, 2011, 12:23 p.m., Thomas Gahr wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102671/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2011, 12:23 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   kdeui/dialogs/kdialog.cpp 0cabb85 
> 
> Diff: http://git.reviewboard.kde.org/r/102671/diff
> 
> 
> Testing
> -------
> 
> Tested successfully with 4.7.0 on Fedora 15 and current master... and on branch KDE/4.7 - as expected everything works fine
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110924/5c92f4d0/attachment.htm>


More information about the kde-core-devel mailing list