Compiz

Merge lp://staging/~compiz-team/compiz/compiz.fix_1159324.1 into lp://staging/compiz/0.9.9

compiz.fix_1159324.1
Merge into 0.9.9

Proposed by Sam Spilsbury on 2013-03-30

Status:

Superseded

Proposed branch:

lp://staging/~compiz-team/compiz/compiz.fix_1159324.1

Merge into:

lp://staging/compiz/0.9.9

Diff against target:

763 lines (+570/-102)

6 files modified

plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h (+28/-0)
plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp (+151/-0)
plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp (+333/-0)
plugins/place/src/place.cpp (+38/-102)
src/window/extents/include/core/windowextents.h (+4/-0)
src/window/extents/src/windowextents.cpp (+16/-0)

To merge this branch:

bzr merge lp://staging/~compiz-team/compiz/compiz.fix_1159324.1

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Daniel van Vugt	2013-03-30	Needs Resubmitting on 2013-04-02
MC Return		Approve on 2013-03-31
Review via email: mp+156245@code.staging.launchpad.net

This proposal has been superseded by a proposal from 2013-04-02.

Commit message

Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.

(LP: #1159324)

Description of the change

Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.

(LP: #1159324)

Revision history for this message

MC Return (mc-return) wrote on 2013-03-30:

A typo in the comment here:

52 + /* left, right, top, bottom target coordinates, clamed to viewport

Revision history for this message

MC Return (mc-return) wrote on 2013-03-31:

In 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h' one newline here is redundant:

8 +
9 +

Other than those 2 minor issues this seems perfect and very useful !

Are you sure it fixes bug #1159324 ? It seems that this is already fixed.

But +1 for the restructuring anyway.

review: Approve

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2013-03-31:

On Sun, Mar 31, 2013 at 8:00 PM, MC Return <email address hidden> wrote:
> Review: Approve
>
> In 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h' one newline here is redundant:
>
> 8 +
> 9 +
>
> Other than those 2 minor issues this seems perfect and very useful !
>
> Are you sure it fixes bug #1159324 ? It seems that this is already fixed.

There was a corner case where it could still happen. Seemed to be
mostly on multimonitor cases. The window would be placed normally
(ok), and then request a new position, and then be constrained as if
it had a border, but that was wrong, because we were never meant to do
that.

>
> But +1 for the restructuring anyway.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1159324.1/+merge/156245
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz.

--
Sam Spilsbury

Revision history for this message

MC Return (mc-return) wrote on 2013-03-31:

>
> There was a corner case where it could still happen. Seemed to be
> mostly on multimonitor cases. The window would be placed normally
> (ok), and then request a new position, and then be constrained as if
> it had a border, but that was wrong, because we were never meant to do
> that.
>
Hmm, that reminds me of a bug I still experience (just with Emerald):

Open any media player-> maximize it -> make it go fullscreen -> restore it
Result:
The window still has no decoration (because it is Unity-maximized),
which is correct, but the window is placed, like it would be decorated,
shifted to the right and down, but with correct window size (making lower
and right part of the window go offscreen...