timerapplet: fix build warning

[zhuyaliang]

@lukefromdc I am not familiar with wayland and have not used it. My question is: can we use only one signal without distinguishing between Display Serves

    g_signal_connect_swapped (applet->applet, "button_press_event",
                              G_CALLBACK (timer_applet_click),
                              applet);

[lukefromdc]

Not without changing x11 behavior. This situation (like the one I had with the trash) involved the x11 code using the button release event to trigger something. That button release event currently does not work in wayland, and worse it triggers the mouse-dodging bug https://github.com/mate-desktop/mate-panel/issues/1156

It would simplify the code to stop using button release in Xorg as well if others here want to do that. I'm pretty sure it was originally used as a simple way to separate the left and right clicks long ago. I've only seen it in two applets from mate-applets so far(timer and trash). By default I have tried to minimize behavior changes in x11.

[zhuyaliang]

The event ->button==1 in the timer_applet_click has been limited to left clicking

static gboolean
timer_applet_click (TimerApplet *applet,  GdkEventButton *event)
{
    if (  event->button == 1)
    {
        if (!applet->active && !applet->pause && applet->elapsed)
            timer_reset_callback (NULL, applet);
        else if (applet->active && !applet->pause)
            timer_pause_callback (NULL, applet);
        else if (!applet->active || applet->pause)
            timer_start_callback (NULL, applet);
    return TRUE;
    }
    else
        return FALSE;
}

So we can simply use a signal of button_press_event

diff --git a/timerapplet/src/timerapplet.c b/timerapplet/src/timerapplet.c
index 4cc464ab..bad95ce8 100644
--- a/timerapplet/src/timerapplet.c
+++ b/timerapplet/src/timerapplet.c
@@ -430,21 +430,9 @@ timer_applet_fill (MatePanelApplet* applet_widget)
                       G_CALLBACK (timer_applet_destroy),
                       applet);

-#ifdef GDK_WINDOWING_X11
-  if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
-    g_signal_connect_swapped (applet->applet, "button-release-event",
-                              G_CALLBACK (timer_applet_click),
-                              applet);
-  else
-    g_signal_connect_swapped (applet->applet, "button_press_event",
-                              G_CALLBACK (timer_applet_click),
-                              applet);
-#else
-    /*if building without x11 support we only have the wayland case*/
     g_signal_connect_swapped (applet->applet, "button_press_event",
                               G_CALLBACK (timer_applet_click),
                               applet);
-#endif

     /* set up context menu */
     applet->action_group = gtk_action_group_new ("Timer Applet Actions");

[lukefromdc]

@raveit65 are you OK with redoing this to use button-pressed on x11 as well as wayland to simplify the code?

[raveit65]

sure

[lukefromdc]

Done, this simplifies the code, uses the left button click in x11 as well as wayland, and of course removing the conditionals removes the build warning caused by the missing black lines after a one line ifclause.

[zhuyaliang]

@lukefromdc Using the middle line - would be more standardized button-press-event

[lukefromdc]

Comment line has to come out, but I do not know what you mean about "middle line"

[lukefromdc]

Comment line removed

[zhuyaliang]

@lukefromdc What I mean is that signal names need to be separated by horizontal lines

For example, button-press-event and button_press_event

[lukefromdc]

button-press-event is not used in the code as just pushed, only button_press_event is

[raveit65]

@zhuyaliang So it should be kebab-case instead of snake_case ? https://www.freecodecamp.org/news/programming-naming-conventions-explained

[zhuyaliang]

So it should be kebab-case instead of snake_case ?

Yes, when using signals, kebab-case is the standard writing, which can be referred to in the implementation of button-press-event

[lukefromdc]

What should we do with this at this point?

[zhuyaliang]

@lukefromdc Does using this format not work? It works well with me

g_signal_connect_swapped (applet->applet, "button-press-event",
                              G_CALLBACK (timer_applet_click),
                              applet);

[lukefromdc]

I don't know enough about formatting conventions to decide that. I am OK with whatever is OK w everyone else and works

[lukefromdc]

Also at the moment I am on the road for a couple days, away from my desktop. If this works in your test I don't have an issue with it.

[raveit65]

With changing signal name to button-press-event the timer notification works out of box under wayfire. So maybe the other PR for mate-notification-daemon isn't needed?

[raveit65]

@lukefromdc I pushed that as single commit to PR. Feel free to delete commit if you do not agree.

[lukefromdc]

I will test build this, no problems on my end unless that test fails. Note that this changes ONE notification, if it makes the notification daemon work out of the box on wayland we need to fix this in our other apps too. Also note that when started the out of the box m-n-d runs for 30 seconds before exiting and will work during that time.

[lukefromdc]

Build works fine, though it did not change wayland behavior with the notification daemon. Thus in at least some cases (you might have something interacting with it and keeping it alive, or just tested before the 30 second timeout ran out.

This change is fine by me and if it's the convention we should follow it. No runtime problems with this,clicking on the applet works fine with this.

This should not affect the notification in any way, as that's not in response to a button-press-event

[raveit65]

I tested it again and found out that starting the wayfire session with gdm display-manager makes the notification run :-) and mate-system-monitor shows me that is it indeed mate-notification-daemon. This behavior is with or without signal name change. With running dbus-launch wayfire i do not have the notification. Any chance to confirm that?

On the other side using gdm is the reason that i do not see all our capplets in m-c-c, ie. mate-volume-control or preferences-applications and some more.

[raveit65]

Also, the notify-send command is working when using a gdm-session and i see an env DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1004/bus. Maybe this makes the notification work? ....only a wild presumption....

[lukefromdc]

Not using gdm as you have to load half of gnome-shell to run it, which is a lot of otherwise unused code outside a GNOME session. I am running wayfire with dbus-launch wayfire from a tty, which would be a different environment.

[lukefromdc]

If running wayfire from sddm, the notification daemon works out of the box, it does NOT when running wayfire from a tty. Note that running from sddm can be a bit buggy: you have to autostart the panel and caja withdbus-launch in ~/.config/wayfire.ini and mate-system-monitor (probably some other stuff too) takes two minutes to start at least if an Xorg session on tty7 is also running. That is because wayfire is not being started with dbus-launch

Worst of all, if you switch TTY's away from the wayland session under SDDM, then back to it, you lose your session, SDDM still running but the session no longer shown. Under a direct TTY start this works fine.

[lukefromdc]

Conclusion: without this, mate-notification-daemon works under some environmental conditions in wayland, with it it should work under all wlroots-based wayland environments.

Down the road, a MATE-wayland session might consist of a symlink to the user's perferred wayland compositor's startup command, plus a .ini file to load the MATE components into it. This can be done NOW, but would not fix the normal packaging systems. A possible workaround would be a PR to wayfire to look in in /etc for a config file installed by a DE, or even a resettabe symlink to one if many are installed e.g if user selects MATE on wayfire, /etc/wayfire.ini would be a symlink to /etc/mate-wayfire.ini

Obviously the dbus-launch issue would have to be resolved. At that point we could possibly bring the timeout in m-n-d back, but only if there is a reason for using it such as reducing resource use. Right now it breaks the notification daemon for at least some use cases in wayland

[lukefromdc]

Anyway, so far as this PR is concerned, it works on my end. Is this ready to go?

[raveit65]

From my point of view as distro maintainer using dbus-launch is not a solution, we need to find out how gdm/sdm or any other display-manager prepared a dbus-session to address notifications. Anyway, this PR is ready to go from my side.