Review Request: KMainWindow::parseGeometry() fails to position with positive coordinates

[Urban Widmark]

> On June 3, 2011, 7:58 p.m., Dawit Alemayehu wrote:
> > Looks good, however should the new if statements have an else that defaulted to previous behavior ? That is should not
> > 
> >   if ( !(m & XValue) )
> >       x = this->x();
> >   if ( !(m & YValue) )
> >       y = this->y();
> > 
> > be 
> > 
> >   x = (!(m & XValue)) ? this->x() : geometry.x();
> >   y = (!(m & YValue)) ? this->y() : geometry.y();
> > 
> > instead or using the else statement if you prefer ?
> >

No, when the flags are set the x/y variables will have been set by XParseGeometry() to the users input. Use of geometry().x() in the original code is wrong.

Hmm, there is another bug in the original code. If you have XNegative set it will use the w value, without checking the WidthValue flag and without having initialized it to any default (eg a geometry string of "-400-300"). Will change it to always set defaults and to use the fact that XParseGeometry() does not change arguments that are not defined in the input string.


- Urban


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


On June 3, 2011, 6:58 p.m., Urban Widmark wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101492/
> -----------------------------------------------------------
> 
> (Updated June 3, 2011, 6:58 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> When an X geometry is given on the command line, parseGeometry() will, for positive positions, use geometry().x()/.y() instead of the x/y value parsed from the string. This causes positive positions to not work. For negative values the string values are used.
> 
> No direct bugs reported on this that I can find, but the odd position behavior is noted in some --geometry related bugs:
> Comment #6, http://bugs.kde.org/show_bug.cgi?id=165355
> http://bugs.kde.org/show_bug.cgi?id=230663
> 
> 
> For the KMainWindow --geometry parsing to work for both size and position, the client application will have to call applyMainWindowSettings() or restoreWindowSize(). The parsing done by KMainWindowPrivate::init() will only set position. Not sure if that is good, as a user of the window I would expect it to use all of the --geometry data at the same time (either on creation or some later call).
> 
> 
> Diffs
> -----
> 
>   kdeui/widgets/kmainwindow.cpp 1d27722 
> 
> Diff: http://git.reviewboard.kde.org/r/101492/diff
> 
> 
> Testing
> -------
> 
> Using a KApplication program with a KMainWindow that also calls applyMainWindowSettings (keditbookmarks), positions verified using xwininfo:
> keditbookmarks --geometry 400x300+100+200
> keditbookmarks --geometry 400x300-100+200
> keditbookmarks --geometry 400x300+100-200
> keditbookmarks --geometry 400x300-100+200
> keditbookmarks --geometry 400x300
> keditbookmarks --geometry +100+200
> 
> Without patch all +coords are replaced by 0.
> Negative positions do not account for window decorations as the size of those are not known. I suspect the user will have to adjust for that in their input.
> 
> 
> Thanks,
> 
> Urban
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110603/7b4265b0/attachment.htm>