Merge lp://qastaging/~thumper/unity/fix_icon_workarea_issues into lp://qastaging/unity

Hi, so this code looks fine, However i'm wondering about testing. we have a system called autopilot but i don't think this is a good fit for that unless there is a nice repeatable methodology for triggering this bug.

At the very least there are manual tests that we have written up in the manual-tests/ directory of unity, if you could add a test in there for this bug that follows the same syntax as the rest that would be great

As a former tester and designer some 20+ years ago I'm familiar with the auto- and manual test concepts as such.

First I would appreciate some complete links for those that I can study and learn from for future use. Not sure what directory "manual-tests/ directory of unity" refers to.

About this bug there is no easy way even to set up a manual test case. That would e.g require some smart patching to catch any improper writing into the struts in Nux. Furthermore, it is obvious that Unity initially sets an improper geometry (nux basewindow defaults) which has to be fixed anyway which my proposed patch suggests.

Basically what happens is that the nux basewindow geometry for e.g the launcher here is treated as a top strut (height=300) in Nux acc to the algorithm at the end of its function XInputWindow::GetStrutsData(). This setting then is not updated until the correct panel height (24) is set. That is, sporadically we can end up with a too small height for the Nautilus workarea dependent on when Nautilus is triggered to draw the workarea.

Then, how to tie and verify this change towards bug #886667?
Well, that's not an easy one, i.e first to show the wrong settings and then to see to that Nautilus catches those. I've no idea how to achieve this with a manual test case.

A final solution for the Unity session must be to implement startup phases acc to:
http://live.gnome.org/SessionManagement/NewGnomeSession
That would at least "hide" bugs like these ;)

What about a workspace test case including four corner desktop icons, one at each corner. Then running a number of restarts, let's say three, to check whether any of those have moved from their original positions. If so the test has failed.

This is more of a general test case for the workspace size consistancy that probably already (should) exist. There are a few other minor workspace issues as well that might be revealed this way.

Unfortunatelly my old ati graphic card broke down so replaced it with an nvidia legacy that require nvidia-173 so waiting for an upgrade to video abi 11 to be able to run unity again.

I see no action on this merge proposal.
Hereby withdrawing my bug assignment.

Please, could you merge this with trunk again?
I get a conflict in in panel/PanelController.cpp due to the latest trunk changes.

I think that this is quite hard to test, however could you at least please include some comments saying why the InputWindowEnableStruts calls should be after the geometry setting?

Thank you, and sorry for the delay.

Thanks for the response.

I haven't been able to compile Unity to trace this issue as I'm not yet familiar with its patch method so have just made a desk check here. So there might be some issues with the patch that I haven't foreseen.

As I recall it the main issue was the setting of the launcher's strut that uses the basewindow default geometry instead of the launcher's dito causing this bug. Concerning the setting of the panel strut this was more kind of a bad coding pratice, i.e doing things in the wrong order not being intuitive but seems to end up to work anyway maybe due to later patchings.

Basically, first update the launcher/panel geometry, then update the corresponding strut for that geometry.

I will do a recheck and come back with more details.

Made a recheck of the panelcontroller part.

window->InputWindowEnableStruts(true)
-------------------------------------
Calls EnableStruts(true) to enable struts for the panel basewindow that in turn calls SetStruts.
SetStruts then uses the current geometry (geometry_) in nux which here still holds the basewindow default geometry (100,100,320,200) that will create a top strut for that basewindow with the height of 300px acc to XInputWindow::GetStrutsData(). This is wrong!

window->SetGeometry(geo)
------------------------
Updates the geometry (geometry_) in nux to the panel's geometry and calls SetStruts() again as struts is now enabled.
SetStruts then uses the current geometry (geometry_) which here now holds the panel's geometry and creates a correct top strut overwriting the previous top strut setting for that basewindow. Fortunally this works as the basewindow geometry also happens to result in a top strut, otherwise we would end up with two struts for that basewindow!

The intension with the fix was to first call SetGeometry to update the basewindow geometry. As struts is not yet enabled InputWindowEnableStruts(true) is then required to enable struts to set the strut for that basewindow. Not sure however what conflict this fix causes.

I think what Marco meant is for you to update your branch to the current unity trunk, since merging from your branch to trunk results in a merge conflict in panel/PanelController.cpp. If that won't be done, the branch - even when approved, cannot get merged into trunk. It has to be made merge-able first.

Thanks, downloaded he current branch and discovered that both the Launcher and PanelController have been moved to other directory. Then the question is how do I update a merge proposal?

First time (new) I did this:

bzr commit
bzr push lp:~bowmore/unity/fix_icon_workarea_issue
bzr lp-open lp:~bowmore/unity/fix_icon_workarea_issues

Can I do the same to replace the current patch with a new one, or?

Hi Bowmore,

Do the following:

bzr merge lp:unity
// fix conflicts
bzr resolve
bzr commit -m "Merge trunk, resolved conflicts."
bzr push # It should remember the previous location

The diff here will automatically update.

I've done the resolution myself, and will resubmit

Thanks for the howto and your resubmit:
https://code.launchpad.net/~thumper/unity/fix_icon_workarea_issues/+merge/116388
Looks fine to me ;-)