Review Request: Prevent resizing panel vertically from breaking the desktop (or bleeding over into positive coords)

[Josh]

> On 2009-02-18 13:24:30, Aaron Seigo wrote:
> > it's an interesting hint as to the problem, but the code is in completely the wrong place. this fix needs to be in Containment, not a specific View subclass.
> 
> Aaron Seigo wrote:
>     can you try this patch:
>     
>     http://pastebin.ca/1341413
>     
>     and see if that fixes it for you? the reason it should fix it is that constraintsEvent is delayed, and that event loop is entered between. this is something that needs to be done synchronously, however. SizeConstraint is triggered by Applet::resizeEvent, so putting this code in resizeEvent is no more or less immune to applets/containments reimplementing resizeEvent and not calling the base implementation.
>     
>     in any case, this fixes it for me, but i'd like some confirmation of that. didn't start a new review request as i'd like to keep this all in one place and oddly rewviewboard doesn't seem to let me post a patch to an existing review?
> 
> wilder wrote:
>     Works for me. Although I did a very similar thing and it didn't solve it. I was probably doing something wrong.
> 
> Josh wrote:
>     Aaron, I have tested your patch with all the scenarios I could think of and it and works.  If you would like I can add your patch onto this review and remove the old one.  Also, on Monday, 2/16, Marco Martin and I came up with a patch that was more like my original idea: to offset the panels position much like the offset that vertical panels have. Although it was just a workaround, he advised me to commit it and CCBUG.  You can see that on the bug report: bug 176280.  I will revert those changes if that is what you desire.   Thanks for taking at look at this and coming up with a much better solution! :)
> 
> Josh wrote:
>     Should we close this review as "submitted" since we are trying to keep all the information in one spot and the actual solution is found here?  Or should we close it with "discarded" because the original patch was not used?  Thanks.

Well that last inquiry might have to be held off for now... It seems I have a problem that wasn't there when I last tested.  Making a horizontal panel into a vertical panel causes some problems.  I will attach a screenshot of what this looks like. Reverting the change fixes it.  

I have also found another way to fix this (probably just workaround :\)... If the previous patch is only used when panels are horizontal, then there are no problems with switching to vertical panels and this patch still fixes the encroaching into positive coordinates problem. An example of this can be seen here: http://pastebin.com/m6ed35c2
Thanks and hopefully this can all be finally laid to rest. :)


- Josh


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


On 2009-02-16 02:16:03, Josh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/101/
> -----------------------------------------------------------
> 
> (Updated 2009-02-16 02:16:03)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch fixes the bug 176280 where resizing a horizontal panel to be larger would break the desktop.  It applies to kdebase/workspace/plasma/shells/desktop/panelview.cpp.  What appears to be the problem is at times panels will begin to bleed over into positive coordinates (they are meant to be in negative coordinates).  A function is added to check for this overflow and fixes it by setting the y position to be -(height_of_panel + 10).  This is a collaboration of a few people and is basically just a hack that has worked on 3 separate machines.  It should help to have more knowledgeable eyes check over it and maybe provide a better way of solving the problem.  Thanks for your time. 
> 
> 
> This addresses bug 176280.
>     https://bugs.kde.org/show_bug.cgi?id=176280
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/shells/desktop/panelview.cpp 925267 
> 
> Diff: http://reviewboard.kde.org/r/101/diff
> 
> 
> Testing
> -------
> 
> Tested with one panel, top and bottom.  Tested with 2 panels, resizing both.
> 
> 
> Screenshots
> -----------
> 
> Switching to vertical panel
>   http://reviewboard.kde.org/r/101/s/25/
> 
> 
> Thanks,
> 
> Josh
> 
>