Merge lp://staging/~azzar1/compiz/fix-874146 into lp://staging/compiz/0.9.9

PASSED: Continuous integration, rev:3522
http://jenkins.qa.ubuntu.com/job/compiz-ci/317/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/317/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/317//rebuild/?

As discussed on IRC test automation would require significant change and
wouldn't effectively test this change.
On 11/12/2012 7:24 AM, "Andrea Azzarone" <email address hidden> wrote:

> Andrea Azzarone has proposed merging lp:~andyrock/compiz/fix-874146 into
> lp:compiz.
>
> Commit message:
> Fix bug 874146 passing valid xwc.width/height through
> validateResizeRequest.
>
> Requested reviews:
> Compiz Maintainers (compiz-team)
> Related bugs:
> Bug #874146 in Compiz: "Multimonitor: New windows open on the wrong
> monitor, Place Plugin settings silently ignored"
> https://bugs.launchpad.net/compiz/+bug/874146
>
> For more details, see:
> https://code.launchpad.net/~andyrock/compiz/fix-874146/+merge/139116
>
> == Problem ==
> Bug #874146: Multimonitor: New windows open on the wrong monitor.
>
> Step to reproduce the bug:
> # Setup a dual monitor with the smaller monitor (yes you need two monitors
> of different sizes) on the left.
> # Open gedit (left monitor)
> # Open a gedit dialog (open with, preferences, etc.)
> # The dialog should open in the right monitor (BUG!!)
>
> == Fix ==
> Pass valid xwc.width/height through validateResizeRequest.
> PlaceWindow::validateResizeRequest does use these values.
> --
> https://code.launchpad.net/~andyrock/compiz/fix-874146/+merge/139116
> Your team Compiz Maintainers is requested to review the proposed merge of
> lp:~andyrock/compiz/fix-874146 into lp:compiz.
>
> === modified file 'src/window.cpp'
> --- src/window.cpp 2012-12-10 03:28:47 +0000
> +++ src/window.cpp 2012-12-10 23:18:21 +0000
> @@ -5429,6 +5429,9 @@
>
> xwcm = adjustConfigureRequestForGravity (&xwc, CWX | CWY, gravity,
> 1);
>
> + xwc.width = priv->serverGeometry.width ();
> + xwc.height = priv->serverGeometry.height ();
> +
> window->validateResizeRequest (xwcm, &xwc, ClientTypeApplication);
>
> CompPoint pos (xwc.x, xwc.y);
>
>
>

The test case sounds more like bug 754508 than bug 874146 (which was meant to describe applications opening on the wrong monitor).

Does this really fix bug 874146? Does it fix both? Or just bug 754508 as the manual test case seems to suggest?

Also, when you say "The dialog should open in the right monitor (BUG!!)", do you mean to say:
  Expected: Dialog should open on the left monitor.
  Observed: Dialog opens on the right monitor.
?

That would sound more correct to me, however I cannot reproduce such a bug right now. The dialogs are correctly opening on the left monitor where the application is.

> The test case sounds more like bug 754508 than bug 874146 (which was meant to
> describe applications opening on the wrong monitor).
>
> Does this really fix bug 874146? Does it fix both? Or just bug 754508 as the
> manual test case seems to suggest?

Should fix both (maybe we can mark them as duplicates).

> Also, when you say "The dialog should open in the right monitor (BUG!!)", do
> you mean to say:
> Expected: Dialog should open on the left monitor.
> Observed: Dialog opens on the right monitor.
> ?
>

That's what I mean sorry.

> That would sound more correct to me, however I cannot reproduce such a bug
> right now. The dialogs are correctly opening on the left monitor where the
> application is.

Is place plugin active? Do you have a multimonitor with monitors of different sizes?

Yes place is active and yes I was using different resolutions as instructed.

I've just changed video cards and will retest. I'll also check if the placement mode is a factor.

> Yes place is active and yes I was using different resolutions as instructed.
>
> I've just changed video cards and will retest. I'll also check if the
> placement mode is a factor.

Make sure that ccsm->General options->Display settings->Overlapping output handling is Smart.

Yes that's set to Smart. And I still can't reproduce any bug. Can't reproduce any regression either.

I give up. Abstain.

+1