Merge lp://qastaging/~renatofilho/compiz/compiz-lp1036542 into lp://qastaging/compiz/0.9.8

Proposed by Renato Araujo Oliveira Filho
Status: Merged
Approved by: Sam Spilsbury
Approved revision: 3309
Merged at revision: 3312
Proposed branch: lp://qastaging/~renatofilho/compiz/compiz-lp1036542
Merge into: lp://qastaging/compiz/0.9.8
Diff against target: 25 lines (+8/-0)
1 file modified
plugins/expo/src/expo.cpp (+8/-0)
To merge this branch: bzr merge lp://qastaging/~renatofilho/compiz/compiz-lp1036542
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
Daniel van Vugt Approve
Review via email: mp+119986@code.qastaging.launchpad.net

Commit message

Ignore mouse press and release if it does not happen inside of the expo
screen. (LP: #1036542)

Description of the change

Ignore mouse press and release if it does not happen inside of the expo screen.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Looks like it should work for Unity. But it might break other /normal/ desktop environments which don't stay visible during expo. Especially with a 2-monitor setup where the expo viewports are rendered right up to the edge of the screen.

I'm not sure if I'm being too pedantic here...

Revision history for this message
Renato Araujo Oliveira Filho (renatofilho) wrote :

> Looks like it should work for Unity. But it might break other /normal/ desktop
> environments which don't stay visible during expo. Especially with a 2-monitor
> setup where the expo viewports are rendered right up to the edge of the
> screen.
>
> I'm not sure if I'm being too pedantic here...

What I understand it that the workArea will be the same size as the rendered viewport and because of that this bug will not happen. Is that correct?
Could you describe the steps to test this specific case?

Thanks

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

What I meant was... try a different desktop environment like Gnome Classic and put a panel down the side of the monitor. Now set up two monitors so that panel is in the middle of them. If you expo now, clicks near the edge of the screen will be ignored despite the fact that you are clicking on the expo windows. But maybe that's too much of an edge case to worry about right now.

Another issue is: What happens if I click inside the workarea, drag a window, but release outside the workarea? Does the window keep dragging? (obviously I haven't tested it yet)

Revision history for this message
Renato Araujo Oliveira Filho (renatofilho) wrote :

>
> Another issue is: What happens if I click inside the workarea, drag a window,
> but release outside the workarea? Does the window keep dragging? (obviously I
> haven't tested it yet)

I did the test here and works fine for me.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Tested. Works for me and the bug is fixed.

It seems my concern about dragging outside the workarea was unfounded. And the issue with expo appearing below the launcher we can ignore for now. Because technically that's a bug and we don't need it to work.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Although, it could still cause strange behaviour with other desktop environments that don't stay on top of expo. But even then, people are unlikely to notice the problem.

Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/47/console reported an error when processing this lp:~renatofilho/compiz/compiz-lp1036542 branch.
Not merging it.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Jenkins died. Keep retying to Approve...

Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/48/console reported an error when processing this lp:~renatofilho/compiz/compiz-lp1036542 branch.
Not merging it.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Tests?

review: Needs Fixing
Revision history for this message
Renato Araujo Oliveira Filho (renatofilho) wrote :

> Tests?

Hi Sam, do you have any idea or example how I can test it?

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Hmm, actually, you'd just be mocking out the compiz API directly. That's testing what it does, not that it works. I'll +1 it for now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches