Merge lp:~macslow/notify-osd/notify-osd.fix-1092905 into lp:notify-osd

[Mirco Müller]

Hardcode the y-coordinate to fix synchronous notifications overlapping panel/indicators when using the 'focus-follow' with multi-monitor setups.

Since 'follow-focus' is the new default for 14.04 LTS, synchronous notifications (e.g. volume up/down, brightness up/down) overlap the panel and indicators on multi-monitor setups. Due to limited time and resources a quick fix was agreed upon for the default Unity-session. Thus the y-coordinate hardcoded for the 'follow-focus' mode now.

Before testing, check what multihead-mode your system is set to, using (in a gnome-terminal)...

  dconf read /apps/notify-osd/multihead-mode

To test the fix, run this branch and change multihead-mode on the fly with (in a gnome-terminal)...

  dconf write /apps/notify-osd/multihead-mode "'focus-follow'"

Now trigger a synchronous notification by e.g. the volume- or screen-brightness-up/down keys on your keyboard. The notification should no longer overlap the panel/indicators.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:482
http://jenkins.qa.ubuntu.com/job/notify-osd-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-armhf-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-i386-ci/14

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notify-osd-ci/18/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Thanks Mirco, that "does the job", though I'm not sure how comfortable I am with setting a fixed value like that, if we do that should we at least restrict the hack to unity session (e.g check for XDG_CURRENT_DESKTOP="Unity")?

Ideally we would fix that bug but I've no idea how difficult that is

[Sebastien Bacher (seb128)]

(thinking about it, the value is also going to cause issue with hi-dpi, unity is going to change the panel size there)

[lp:~macslow/notify-osd/notify-osd.fix-1092905]

483. By Mirco Müller on 2014-01-24

Make the fix for synchronous notifications overlapping panel/indicators when using the 'focus-follow' with multi-monitor setups more robust.

[Mirco Müller (macslow)]

I've pushed a more robust fix.

[Sebastien Bacher (seb128)]

Thanks

> !g_strcmp0("Unity", g_environ_getenv (g_get_environ (), "XDG_CURRENT_DESKTOP")))

that seems a bit complicated, we usually use "!g_strcmp0 (g_getenv ("XDG_CURRENT_DESKTOP"), "Unity")" (one less call)

I'm also not sure to understand the y computation ... do we have somebody knowing that code a bit that could review that? (I still don't understand why we can't just use the same computation than the no-focus-follow case since that one is correct, the panel size doesn't change between monitors)

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:483
http://jenkins.qa.ubuntu.com/job/notify-osd-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-armhf-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-i386-ci/15

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notify-osd-ci/19/rebuild

review: Approve (continuous-integration)

[Mirco Müller (macslow)]

The code for the multihead-mode is from a time, when unity wasn't the default desktop-environment. We had plain compiz back then. The panel could appear on only on screen in a multi-monitor setup. It was wrong to assume the panel would be cloned on every screen, like we have now with the unity-plugin.

I don't want to rewrite all of defaults_get_top_corner() now.

[Sebastien Bacher (seb128)]

oh, in that context it makes sense ... if you fix the getenv thing the change looks fine to me then, thanks!

[lp:~macslow/notify-osd/notify-osd.fix-1092905]

484. By Mirco Müller on 2014-01-24

Avoid superfluous recomputation of y-coordinate.

[Sebastien Bacher (seb128)]

Great, the new version looks better to me (and works fine as well), thanks for the work there ;-)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:484
http://jenkins.qa.ubuntu.com/job/notify-osd-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-armhf-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-i386-ci/16

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notify-osd-ci/20/rebuild

review: Approve (continuous-integration)

[Iain Lane (laney)]

Seb asked me to test this, here are my results

Laptop running a gnome-panel based session, single monitor (internal panel): Synchronous notifications still overlap the panel.

Desktop, unity, multiple monitors: Synchronous notifications don't appear at all on the left (primary) montior, when a window there has focus. They appear in the correct place on the secondary one.

review: Needs Fixing

[Sebastien Bacher (seb128)]

Confirmed, if you use 2 monitors with different resolution, bubbles get shifted to the top on the smaller resolution one (they can end up offscreen is the resolution is low enough)

[Sebastien Bacher (seb128)]

setting as rejected, it's superseeded by https://code.launchpad.net/~larsu/notify-osd/always-use-workarea/+merge/206145

Unmerged revisions

484. By Mirco Müller on 2014-01-24

Avoid superfluous recomputation of y-coordinate.

483. By Mirco Müller on 2014-01-24

Make the fix for synchronous notifications overlapping panel/indicators when using the 'focus-follow' with multi-monitor setups more robust.

482. By Mirco Müller on 2014-01-24

Hardcode the y-coordinate to fix synchronous notifications overlapping panel/indicators when using the 'focus-follow' with multi-monitor setups.

=== modified file 'src/defaults.c'
--- src/defaults.c	2012-08-29 22:05:41 +0000
+++ src/defaults.c	2014-01-24 14:12:44 +0000
@@ -2279,6 +2279,8 @@
 	         rect.width,
 	         rect.height);
 
+	g_object_get (self, "desktop-top", &rect.y, NULL);
+
 	/* Position the top left corner of the stack. */
 	if (has_panel_window &&
 	    panel_monitor == monitor)