After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770795 - Please port gnome-window-applets into gnome-applets
Please port gnome-window-applets into gnome-applets
Status: RESOLVED FIXED
Product: gnome-applets
Classification: Other
Component: general
3.20.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-applets Maintainers
gnome-applets Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-03 14:24 UTC by Khurshid Alam
Modified: 2017-04-02 08:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Integration of window-title and window-buttons to gnome-applets (791.14 KB, patch)
2016-09-20 00:56 UTC, Gabriel Finkelstein
none Details | Review
Import of two window applets (854.39 KB, patch)
2016-09-24 17:53 UTC, Gabriel Finkelstein
none Details | Review
Build files for window-buttons and window-title (10.23 KB, patch)
2016-09-24 17:54 UTC, Gabriel Finkelstein
none Details | Review
Modifications in source files for window-buttons (132.92 KB, patch)
2016-09-24 17:54 UTC, Gabriel Finkelstein
none Details | Review
Modifications in source files for window-title (80.07 KB, patch)
2016-09-24 17:54 UTC, Gabriel Finkelstein
none Details | Review
Some translation modifications for window-buttons and window-title (1.62 KB, patch)
2016-09-24 17:54 UTC, Gabriel Finkelstein
none Details | Review
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version (127.67 KB, patch)
2016-11-20 16:57 UTC, Gabriel Finkelstein
none Details | Review
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version (85.82 KB, patch)
2016-11-20 16:57 UTC, Gabriel Finkelstein
none Details | Review
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version (83.30 KB, patch)
2017-03-25 19:42 UTC, Gabriel Finkelstein
none Details | Review
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version (69.32 KB, patch)
2017-03-25 19:44 UTC, Gabriel Finkelstein
none Details | Review
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version (83.25 KB, patch)
2017-03-25 19:49 UTC, Gabriel Finkelstein
none Details | Review
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version (69.63 KB, patch)
2017-03-25 19:50 UTC, Gabriel Finkelstein
none Details | Review
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version (54.19 KB, patch)
2017-03-25 19:57 UTC, Gabriel Finkelstein
none Details | Review
window-buttons: declare functions as static (2.55 KB, patch)
2017-03-27 23:03 UTC, Gabriel Finkelstein
none Details | Review
window-title: inherit fg color of theme (3.84 KB, patch)
2017-03-27 23:37 UTC, Gabriel Finkelstein
needs-work Details | Review
window-title: use text color of panel (4.33 KB, patch)
2017-03-28 22:12 UTC, Gabriel Finkelstein
none Details | Review

Description Khurshid Alam 2016-09-03 14:24:17 UTC
Please port gnome-window-applets which once was a part of gnome-applets. I can not use it beyond trusty (Ubuntu 14.04, Gtk-3.10).

The source can be found here: http://www.ubuntuupdates.org/package/webupd8/trusty/main/base/gnome-window-applets

Thanks.
Comment 1 Alberts Muktupāvels 2016-09-05 11:08:13 UTC
Can you give more info?

- Is there upstream git or something like that for gnome-windows-applets? Where should I submit patches if I plan to do that?

- If it was part of gnome-applets then can you give links with info why it was dropped in first place? Why? When? What version?
Comment 2 Khurshid Alam 2016-09-11 06:28:06 UTC
Well I am not sure about the origin. But I saw this in WIP branch during gnome-2 era. After Gnome dropped panel it was picked up by community and was being hosted on gnome-look. But now they don't host anything other than themes. 

If it is required I can upload original tar from launchpad and upload to github.

Source: https://launchpad.net/~nilarimogard/+archive/ubuntu/webupd8/+files/gnome-window-applets_0.3.orig.tar.gz

Also, it seems someone already started working: 

https://code.launchpad.net/~gabrielfinkelstein/+junk/gnome-window-applets
Comment 3 Gabriel Finkelstein 2016-09-11 15:22:16 UTC
The original applet comes from
https://www.gnome-look.org/content/show.php?content=103732

Then it was maintained until Utopic by Eugene San and Alin Andrei (from Webupd8)
https://launchpad.net/~eugenesan/+archive/ubuntu/ppa
https://launchpad.net/~nilarimogard/+archive/ubuntu/webupd8

I don't think it was ever part of gnome-applets.
Comment 4 Alberts Muktupāvels 2016-09-11 17:03:16 UTC
Ok, but do we need to merge it into gnome-applets? Or it will be maintained as separated project?
Comment 5 Gabriel Finkelstein 2016-09-11 17:54:15 UTC
Well, I can't maintain it, I barely know what I'm doing. Ideally, yes, it would be great to add them to gnome-applets.
But if you feel it's too much work to maintain, we could maybe look around for someone to step up. For starters, we could try and contact Andrej Belcijan (the original developer), Eugene San or Alin Andrei.
Comment 6 Sebastian 2016-09-13 21:47:16 UTC
I see that those applets are registered as GPLv3 (see COPYING) but the headers in the individual source files are mostly GPLv2 (or later) with the exception that gnome-title/windowtitle.c is GPLv3. If we want to merge that into gnome-applets I would prefer to do it as GPLv2 (or later). Otherwise gnome-applets would implicitly become GPLv3 only, but for that to work we must find out why that one file is GPLv3 or contact the author and ask him to give us permission to re-license it to GPLv2.

Assuming we get that resolved then I see no problem to merge those applets into gnome-applets.

Looking at the source I see that gnome-window-applets are actually two applets, I suggest we integrate them into gnome-applets as "window-title" and "window-button". Each applet should be converted to in-process before that (see https://git.gnome.org/browse/gnome-applets/commit/?id=35f46231840c586288f40cf99ad692d97ad0a681) for an example.

It would be great if someone of you could come up with a patch to merge each of those two applets. I can provide help if you need and I can review the patches, but right now I do not have time create a patch my self.
Comment 7 Gabriel Finkelstein 2016-09-20 00:56:07 UTC
Created attachment 335883 [details] [review]
Integration of window-title and window-buttons to gnome-applets

I made a patch that merges both applets into gnome-applets. I also converted them to in-process and fixed the "Hide Compiz decoration for maximized windows" which I previously did not know how to port to GSettings.

I created the patch using `git diff origin/master --binary`, I don't know if that's the right way...
Comment 8 Sebastian 2016-09-20 11:07:57 UTC
Hi Gabriel,

thank you very much. I just gave your patch a quick look. I have no time today to do a thorough review, but I will try to do that this week or on the weekend.

One thing I noticed though is that there seem to be no translation files and no changes to po/POTFILES.in in your patch.

Also, to make the review easier, could you please split your patch into several smaller patches. Preferably something like this:

A separate set of patches for each applet, with patches split something like this:
 * One patch, that adds the build system parts
 * One patch, that adds the source files
 * One patch, that adds translation files
 * Maybe also split the pixmaps parts into a separate patch

With a simple git rebase that should be easy to achieve.

Btw, if you are not yet using git-bz I recommend you to take a look at that, it makes it quit easy to upload patches to bugzilla. See here: https://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/ and here: httpshttp://git.fishsoup.net/man/git-bz.html

Cheers
Sebastian
Comment 9 Alberts Muktupāvels 2016-09-20 11:20:14 UTC
First patch should include unmodified original applet code:
- download gnome-window-applets-0.3.tar.gz
- copy gnome-buttons as window-buttons
- copy gnome-title as window-title

commit title:
import two window applets

commit message:
Add short info about applets and link from were applets was copied (https://www.gnome-look.org/content/show.php?content=103732).

--

Then I think it is ok to do everything that is needed to get applets running as second patch. Original applet does not include translations, but po/POTFILES.in must be updated.

--

But still we have problem with GPLv3...
Comment 10 Gabriel Finkelstein 2016-09-24 17:53:54 UTC
Created attachment 336196 [details] [review]
Import of two window applets

GNOME Panel applets substitute the maximized window title and buttons. Window Title and Window Buttons are essentially controls for windows that are placed on the Panel instead of a window. They also provide a clever way to increase vertical screen space.
https://www.gnome-look.org/content/show.php?content=103732
Comment 11 Gabriel Finkelstein 2016-09-24 17:54:00 UTC
Created attachment 336197 [details] [review]
Build files for window-buttons and window-title
Comment 12 Gabriel Finkelstein 2016-09-24 17:54:05 UTC
Created attachment 336198 [details] [review]
Modifications in source files for window-buttons
Comment 13 Gabriel Finkelstein 2016-09-24 17:54:10 UTC
Created attachment 336199 [details] [review]
Modifications in source files for window-title
Comment 14 Gabriel Finkelstein 2016-09-24 17:54:15 UTC
Created attachment 336200 [details] [review]
Some translation modifications for window-buttons and window-title
Comment 15 Gabriel Finkelstein 2016-09-24 18:00:39 UTC
I hope the patches are OK.

I emailed the original developer about the licensing issue a few days ago. I'm still waiting for a reply.
Comment 16 Alberts Muktupāvels 2016-09-28 19:27:15 UTC
(In reply to Gabriel Finkelstein from comment #15)
> I emailed the original developer about the licensing issue a few days ago.
> I'm still waiting for a reply.

Have you got reply or still waiting? We need to get permission to use his work with GPLv2+ / change license header. If he allows that then please ask him to post that here.
Comment 17 Khurshid Alam 2016-09-28 20:03:05 UTC
I already emailed Eugene San and Alin Andrei to comment here.
Comment 18 eugenesan 2016-09-28 21:29:18 UTC
Great work!
I am all in for the integration.

Is there a git tree with the integrated applets?
It would much easier to review the patchset...

For now I would recommend the following changes:
1. Rename the widgets as follows "window-title" -> "windowtitle" for consistency with windowpicker.
2. Remove ancient themes to reduce amount of blob introduced with the integration.
3. Restore Metacity's hidden maximized-titlebar support. Changes required in Metacity and example of how it can be triggered (on example of windowpicker) can be found at: https://mail.gnome.org/archives/gnome-flashback-list/2015-December/msg00017.html
Comment 19 Gabriel Finkelstein 2016-09-28 21:59:34 UTC
No, I still haven't received a reply. I just posted a message in https://www.gnome-look.org/content/show.php?content=103732 . Maybe he'll read it...
Comment 20 Sebastian 2016-09-29 09:40:50 UTC
> 1. Rename the widgets as follows "window-title" -> "windowtitle" for consistency with windowpicker.

Please keep "window-title", I know its not consistent with window picker, but I recently changed a lot of folder named to use the style using a hyphen between name parts. I still plan to rename "windowpicker" to "window-picker", then it will be consistent again.
Comment 21 andrejx 2016-11-04 05:55:21 UTC
Hello everyone. Original developer of Gnome Window Applets here. Until Gabriel emailed me a while ago I thought this project was pretty much dead. Glad to see it being revived!

First of all, the GPL issue. The project was supposed to be GPLv3 simply because I had to pick something, but I don't mind officially making it GPLv2.

I see lots of progress has been made here already. I have been out of the loop for quite some time, so I don't know if I can really offer any substantial help (you guys seem to have it all covered pretty well), but do let me know if you need my assistance. I'll do what I can to help.
Comment 22 Alberts Muktupāvels 2016-11-09 18:18:31 UTC
I have just pushed to master two commits:
- first that simply adds both applets without any modifications
- second that changes license header to GPLv2+

Now I would like to see two patches:
- window-buttons: ...
- window-title: ...

Each patch must include everything that is needed to get applet build and work. Gabriel, can you do it?

Before attaching patches please make sure that distcheck works:
make distcheck
Comment 23 Gabriel Finkelstein 2016-11-10 02:52:22 UTC
I'm gonna look at it over the weekend. 

Thanks Andrej for stopping by, and thank you very much for these great applets. I still use them every day!!
Comment 24 Gabriel Finkelstein 2016-11-20 02:11:54 UTC
After fighting all week to install a Ubuntu version with gnome 3.22 in VirtualBox, I finally got it to compile. The only problem I have now is that make distcheck returns:

make[4]: Entering directory '/home/gabriel/Desktop/gnome-applets2/cpufreq/src/cpufreq-selector'
make[4]: *** No rule to make target 'cpufreq-selector-service-glue.h', needed by 'distdir'.  Stop.
make[4]: Leaving directory '/home/gabriel/Desktop/gnome-applets2/cpufreq/src/cpufreq-selector'
Makefile:865: recipe for target 'distdir' failed
make[3]: *** [distdir] Error 1
make[3]: Leaving directory '/home/gabriel/Desktop/gnome-applets2/cpufreq/src'
Makefile:682: recipe for target 'distdir' failed
make[2]: *** [distdir] Error 1
make[2]: Leaving directory '/home/gabriel/Desktop/gnome-applets2/cpufreq'
Makefile:704: recipe for target 'distdir' failed
make[1]: *** [distdir] Error 1
make[1]: Leaving directory '/home/gabriel/Desktop/gnome-applets2'
Makefile:802: recipe for target 'dist' failed
make: *** [dist] Error 2

I have no idea what that means...
Comment 25 Alberts Muktupāvels 2016-11-20 14:50:27 UTC
Ok, forget about distcheck. Do you have patches for master?
Comment 26 Gabriel Finkelstein 2016-11-20 16:57:02 UTC
Created attachment 340362 [details] [review]
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 27 Gabriel Finkelstein 2016-11-20 16:57:50 UTC
Created attachment 340363 [details] [review]
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 28 Khurshid Alam 2016-11-20 17:55:23 UTC
@Gabriel
Is gtk-3.22 hard-coded dependency? Or can it also be compiled for gtk-3.20?
Comment 29 Alberts Muktupāvels 2016-11-20 17:55:59 UTC
Review of attachment 340362 [details] [review]:

::: configure.ac
@@ +747,3 @@
+  window-buttons/Makefile
+  window-buttons/pixmaps/Makefile
+  window-buttons/themes/Makefile

please move before windowpicker.

@@ +792,3 @@
 echo "    trash .........................: always"
 echo "    windowpicker ..................: always"
+echo "    window-buttons ................: always"

Same, please move before window picker.

::: po/POTFILES.in
@@ +121,3 @@
 [type: gettext/glade]trash/src/trash-empty.ui
 [type: gettext/glade]trash/src/trash-menu.xml
+[type: gettext/ini]window-buttons/org.gnome.applets.WindowButtonsApplet.panel-applet.in.in

Please do not rename this file, this is unneeded change to get applets build. Also we have two other applets that also use org.gnome.panel prefix so this is not problem.

@@ +122,3 @@
 [type: gettext/glade]trash/src/trash-menu.xml
+[type: gettext/ini]window-buttons/org.gnome.applets.WindowButtonsApplet.panel-applet.in.in
+window-buttons/preferences.c

preferences.c does not have translatable text, no need to add it here.

::: window-buttons/Makefile.am
@@ +1,1 @@
+APPLET_LOCATION = $(pkglibdir)/$(LIBPANEL_APPLET_API_VERSION)/libwindow-buttons-applet.so

We dont have LIBPANEL_APPLET_API_VERSION, remove it - not needed.

::: window-buttons/org.gnome.gnome-applets.windowbuttons.gschema.xml.in.in
@@ +1,2 @@
+<schemalist>
+  <schema id="org.gnome.gnome-applets.windowbuttons">

org.gnome.gnome-applets.window-buttons, same with filename.

::: window-buttons/preferences.c
@@ +27,2 @@
 /* prototypes */
+void windowbuttons_cb_only_maximized (GtkButton *, WBApplet *);

Why did you rename these functions? It is unneeded change to get applets build.

::: window-buttons/theme.c
@@ +1,3 @@
 #include "theme.h"
 
+void windowbuttons_loadThemeComboBox(GtkComboBox *, gchar *);

Same, please don't rename functions.

::: window-buttons/windowbuttons.c
@@ +27,3 @@
 /* Prototypes */
 //static void applet_change_background (PanelApplet *, PanelAppletBackgroundType, GdkColor *, GdkPixmap *);
+static void windowbuttons_active_workspace_changed (WnckScreen *, WnckWorkspace *, WBApplet *);

Again, please don't rename functions.

::: window-buttons/windowbuttons.ui
@@ +176,3 @@
                         </child>
                         <child>
+                          <object class="GtkButton" id="btn-focused-normal-minimize">

Why this change?

::: window-title/org.gnome.panel.applet.WindowTitleAppletFactory.service.in
@@ -1,3 @@
-[D-BUS Service]
-Name=org.gnome.panel.applet.WindowTitleAppletFactory
-Exec=@LIBEXECDIR@/windowtitle

This change should not be in this patch.
Comment 30 Alberts Muktupāvels 2016-11-20 17:58:45 UTC
(In reply to Khurshid Alam from comment #28)
> @Gabriel
> Is gtk-3.22 hard-coded dependency? Or can it also be compiled for gtk-3.20?

GNOME Applets does not depend on GTK+ 3.22, but depends on gnome-panel 3.22...
Comment 31 Alberts Muktupāvels 2016-11-20 18:02:33 UTC
Review of attachment 340363 [details] [review]:

::: po/POTFILES.in
@@ +133,3 @@
 windowpicker/src/wp-task-title.c
+[type: gettext/ini]window-title/org.gnome.applets.WindowTitleApplet.panel-applet.in.in
+window-title/preferences.c

Just like in previous patch:
- don't rename file
- preferences.c is not needed here

::: window-title/Makefile.am
@@ +1,1 @@
+APPLET_LOCATION = $(pkglibdir)/$(LIBPANEL_APPLET_API_VERSION)/libwindow-title-applet.so

Remove LIBPANEL_APPLET_API_VERSION.

::: window-title/preferences.c
@@ +27,3 @@
 /* prototypes */
+void windowtitle_cb_only_maximized (GtkButton *, WTApplet *);
+void windowtitle_cb_click_effect (GtkButton *, WTApplet *);

Please dont rename functions.

::: window-title/windowtitle.ui
@@ +80,3 @@
                                 <property name="can_focus">False</property>
                                 <child>
+                                  <object class="GtkCheckButton" id="swap-order">

This is unneeded change, please dont change.
Comment 32 Gabriel Finkelstein 2016-11-21 00:07:18 UTC
> ::: window-buttons/preferences.c
> @@ +27,2 @@
>  /* prototypes */
> +void windowbuttons_cb_only_maximized (GtkButton *, WBApplet *);

> Why did you rename these functions? It is unneeded change to get applets build.

When moving the applets to in-process I noticed there were conflicts with functions that had the same name. Calling certain function in one applet (for example properties_cb or loadPreferences) would call the function of the same name defined in the other applet, and therefore crash/throw errors. So I assumed function names had to be unique. Prefixing them with the applet name solved the problem. I was also afraid that the same would happen with the rest of the applets, so I prefixed all functions.


> ::: window-buttons/windowbuttons.ui
> @@ +176,3 @@
>                          </child>
>                          <child>
> +                          <object class="GtkButton" id="btn-focused-normal-minimize">

> Why this change?

Because the code expects that id to be the same as the gsettings key.


> ::: window-title/org.gnome.panel.applet.WindowTitleAppletFactory.service.in
> @@ -1,3 @@
> -[D-BUS Service]
> -Name=org.gnome.panel.applet.WindowTitleAppletFactory
> -Exec=@LIBEXECDIR@/windowtitle

> This change should not be in this patch.

I thought that file was needed only for out-of-process applets, therefore removed it when switching to in-process. Is it for something else?


> ::: window-title/windowtitle.ui
> @@ +80,3 @@
>                                  <property name="can_focus">False</property>
>                                  <child>
> +                                  <object class="GtkCheckButton" id="swap-order">

> This is unneeded change, please dont change.

The code used the same constant for referencing the id and for the gsettings key. So I either made this simple change, or I had to split the constant in two and identify where each constant was required.


> @Gabriel
> Is gtk-3.22 hard-coded dependency? Or can it also be compiled for gtk-3.20?
The code works with 3.20, but the patches are made for the latest revision, which uses 3.22. You may have to manually apply some parts of the patch.
Comment 33 Alberts Muktupāvels 2016-11-26 22:07:51 UTC
(In reply to Gabriel Finkelstein from comment #32)
> > ::: window-buttons/preferences.c
> > @@ +27,2 @@
> >  /* prototypes */
> > +void windowbuttons_cb_only_maximized (GtkButton *, WBApplet *);
> 
> > Why did you rename these functions? It is unneeded change to get applets build.
> 
> When moving the applets to in-process I noticed there were conflicts with
> functions that had the same name. Calling certain function in one applet
> (for example properties_cb or loadPreferences) would call the function of
> the same name defined in the other applet, and therefore crash/throw errors.
> So I assumed function names had to be unique. Prefixing them with the applet
> name solved the problem. I was also afraid that the same would happen with
> the rest of the applets, so I prefixed all functions.

I would say that this is bug in gnome-panel. Can you try to rebuild gnome-panel?

https://git.gnome.org/browse/gnome-panel/tree/gnome-panel/libpanel-applet-private/panel-applets-manager-dbus.c#n426

Change this line:
info->module = g_module_open (info->location, G_MODULE_BIND_LAZY);

to:
info->module = g_module_open (info->location, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);

> > ::: window-buttons/windowbuttons.ui
> > @@ +176,3 @@
> >                          </child>
> >                          <child>
> > +                          <object class="GtkButton" id="btn-focused-normal-minimize">
> 
> > Why this change?
> 
> Because the code expects that id to be the same as the gsettings key.

I do prefer `-` over `_`, but this can cause problems in future. For example if we will convert to template. 

> > ::: window-title/org.gnome.panel.applet.WindowTitleAppletFactory.service.in
> > @@ -1,3 @@
> > -[D-BUS Service]
> > -Name=org.gnome.panel.applet.WindowTitleAppletFactory
> > -Exec=@LIBEXECDIR@/windowtitle
> 
> > This change should not be in this patch.
> 
> I thought that file was needed only for out-of-process applets, therefore
> removed it when switching to in-process. Is it for something else?

It is not needed, yes. But you are removing file from window-title. It simply needs to be part of second patch not this.

> > ::: window-title/windowtitle.ui
> > @@ +80,3 @@
> >                                  <property name="can_focus">False</property>
> >                                  <child>
> > +                                  <object class="GtkCheckButton" id="swap-order">
> 
> > This is unneeded change, please dont change.
> 
> The code used the same constant for referencing the id and for the gsettings
> key. So I either made this simple change, or I had to split the constant in
> two and identify where each constant was required.
> 
> 
> > @Gabriel
> > Is gtk-3.22 hard-coded dependency? Or can it also be compiled for gtk-3.20?
> The code works with 3.20, but the patches are made for the latest revision,
> which uses 3.22. You may have to manually apply some parts of the patch.
Comment 34 Alberts Muktupāvels 2017-03-21 16:50:02 UTC
Any news?
Comment 35 Gabriel Finkelstein 2017-03-25 19:42:56 UTC
Created attachment 348716 [details] [review]
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 36 Gabriel Finkelstein 2017-03-25 19:44:46 UTC
Created attachment 348717 [details] [review]
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 37 Gabriel Finkelstein 2017-03-25 19:49:43 UTC
Created attachment 348719 [details] [review]
window-buttons: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 38 Gabriel Finkelstein 2017-03-25 19:50:27 UTC
Created attachment 348720 [details] [review]
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 39 Gabriel Finkelstein 2017-03-25 19:57:45 UTC
Created attachment 348722 [details] [review]
window-title: switched to in-process, gsettings, and adapted to new libpanel-applet version
Comment 40 Gabriel Finkelstein 2017-03-25 20:01:16 UTC
Attached new patches with requested modifications. Sorry about the mess, I can't delete the failed patches.


> I would say that this is bug in gnome-panel. Can you try to rebuild
> gnome-panel?
> 
> https://git.gnome.org/browse/gnome-panel/tree/gnome-panel/libpanel-applet-
> private/panel-applets-manager-dbus.c#n426
> 
> Change this line:
> info->module = g_module_open (info->location, G_MODULE_BIND_LAZY);
> 
> to:
> info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> G_MODULE_BIND_LOCAL);
> 

I tried but I'm probably doing something wrong because my compiled gnome-panel starts misbehaving. The themeing is wrong, some applets disappeared, and window-buttons and window-title are missing, so I can't even test them. I'm gonna retry with a clear head later.
Comment 41 Alberts Muktupāvels 2017-03-25 20:35:19 UTC
You don't need to delete old patches, just mark as obsolete, as you did.

What combination do you use? Are you building gnome-panel & gnome-applets from master? At this point it is better to use gnome-3-24 branches, but master should work too...

Did quick look and now patches looks better. I will test myself when I will be back at my main pc - tomorrow or maybe monday (or in few hours if my mobile internet will be fast enough). I want to get this in 3.24.x release, thanks for updating patches.
Comment 42 Gabriel Finkelstein 2017-03-25 22:57:00 UTC
I tested with master and gnome-3-24. I'm probably compiling it wrong, it has something to do with paths. Anyway, with a couple of symlinks I finally made it work.


Applying the change you proposed:

> info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> G_MODULE_BIND_LOCAL);

Does indeed solve the problem.
The only problem that remains is that all applets use the same "About" dialog, as if that dialog was stuck. (i.e. after opening the "About" dialog of applet X, every other applet will show the "About" dialog of applet X).
But I think this is a gnome-panel bug, because I can reproduce it event with the official builds.
Comment 43 Alberts Muktupāvels 2017-03-26 17:23:48 UTC
How much did you test this?

1) window-buttons does not work for me... I can add applet, but clicking on minimize and maximize does nothing. Trying to open I get warning about invalid cast from GtkBuilder to GtkWindow. :(

2) window-title crash panel with schema 'org.gnome.gnome-applets.window-title' does not contain a key named 'button-minimize-hidden'. :(

3) Then there is something wrong with POTFILES.skip, it adds warning:
\ No newline at end of file
(both patches)

4) decorPluginInstalled in window-buttons applet is used only in extern.c. It must be static function and should be moved before first usage.

Do you have enough time in next few days to work on this? Asking to know if I should wait at least few more days before making 3.24 release.

P.S. I think I was able reproduce About dialog problem, but have not time to debug it right now. :( Feel free to open new bug about that.
Comment 44 Gabriel Finkelstein 2017-03-26 20:23:22 UTC
> 1) window-buttons does not work for me... I can add applet, but clicking on
> minimize and maximize does nothing. Trying to open I get warning about
> invalid cast from GtkBuilder to GtkWindow. :(

window-buttons handles only maximized windows. I you have no maximized windows then it does nothing. Perhaps you should enable the preference "Behavior" > "Hide when no active maximized window".

I don't see the warning you mention. Although I do see other warnings :s


> 2) window-title crash panel with schema
> 'org.gnome.gnome-applets.window-title' does not contain a key named
> 'button-minimize-hidden'. :(

That's because of the bug in gnome-panel you mentioned. It is fixed with:
> info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> G_MODULE_BIND_LOCAL);


> 3) Then there is something wrong with POTFILES.skip, it adds warning:
> \ No newline at end of file
> (both patches)

Sorry, did not know a newline at the end was required.


> 4) decorPluginInstalled in window-buttons applet is used only in extern.c.
> It must be static function and should be moved before first usage.
> 
> Do you have enough time in next few days to work on this? Asking to know if
> I should wait at least few more days before making 3.24 release.

Ok, here's the thing. I'm not a C developer and definitely not a GTK developer, so I barely know what I'm doing. Most of the things I've done in these patches I simply copied from other applets, and even that was really hard for me. I just ported these applets in the hope that someone would pick them up, because I'm sure they need a lot of work, which I'm not capable of doing. So if someone reading this would like to take over, please do. Still, I'm gonna help as much as I can, although I probably can help out on weekends only.

When you say you want to include them in the 3.24 release, how ready do these applets need to be? Is it enough for the applets to "just work", or do they must have absolutely no warnings, translations, refactored/modernized code, etc?
Comment 45 Alberts Muktupāvels 2017-03-27 09:03:03 UTC
(In reply to Gabriel Finkelstein from comment #44)
> > 1) window-buttons does not work for me... I can add applet, but clicking on
> > minimize and maximize does nothing. Trying to open I get warning about
> > invalid cast from GtkBuilder to GtkWindow. :(
> 
> window-buttons handles only maximized windows. I you have no maximized
> windows then it does nothing. Perhaps you should enable the preference
> "Behavior" > "Hide when no active maximized window".

Oh, ok. I have never used that applet... It should probably hide by default or better to change look so it is clearly visible that buttons are disabled... 

> I don't see the warning you mention. Although I do see other warnings :s

I can not open preferences... Right click on applet -> preferences does nothing and if gnome-panel is restarted form terminal it also prints warning about invalid cast.

> > 2) window-title crash panel with schema
> > 'org.gnome.gnome-applets.window-title' does not contain a key named
> > 'button-minimize-hidden'. :(
> 
> That's because of the bug in gnome-panel you mentioned. It is fixed with:
> > info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> > G_MODULE_BIND_LOCAL);

No, at least I fail to see how that could be related.

Did quick grep... Key 'button-minimize-hidden' is available in *windows-buttons.gschema.xml not in *window-title.gschema.xml. So why window-title tries to access key from window-buttons applet schema? Typo / copy&paste error?

Also noticed that patch does not delete old gconf schemas:
windowtitle.schemas.in
windowbuttons.schemas.in

> > 3) Then there is something wrong with POTFILES.skip, it adds warning:
> > \ No newline at end of file
> > (both patches)
> 
> Sorry, did not know a newline at the end was required.

Required, also adds uneeded diff to patch... So, please make sure that patch does not remove newlines.

> > 4) decorPluginInstalled in window-buttons applet is used only in extern.c.
> > It must be static function and should be moved before first usage.
> > 
> > Do you have enough time in next few days to work on this? Asking to know if
> > I should wait at least few more days before making 3.24 release.
> 
> Ok, here's the thing. I'm not a C developer and definitely not a GTK
> developer, so I barely know what I'm doing. Most of the things I've done in
> these patches I simply copied from other applets, and even that was really
> hard for me. I just ported these applets in the hope that someone would pick
> them up, because I'm sure they need a lot of work, which I'm not capable of
> doing. So if someone reading this would like to take over, please do. Still,
> I'm gonna help as much as I can, although I probably can help out on
> weekends only.

Just move this function before first usage and add static keyword.

> When you say you want to include them in the 3.24 release, how ready do
> these applets need to be? Is it enough for the applets to "just work", or do
> they must have absolutely no warnings, translations, refactored/modernized
> code, etc?

They must work. But in long term I would want that warnings and deprecations are fixed, but this is problem in all applets not only in these.

So preferences must be fixed in windows-buttons and gsettings schema bug/problem in window-title.
Comment 46 Gabriel Finkelstein 2017-03-27 15:53:29 UTC
(In reply to Alberts Muktupāvels from comment #45)
> (In reply to Gabriel Finkelstein from comment #44)
> > > 1) window-buttons does not work for me... I can add applet, but clicking on
> > > minimize and maximize does nothing. Trying to open I get warning about
> > > invalid cast from GtkBuilder to GtkWindow. :(
> > 
> > window-buttons handles only maximized windows. I you have no maximized
> > windows then it does nothing. Perhaps you should enable the preference
> > "Behavior" > "Hide when no active maximized window".
> 
> Oh, ok. I have never used that applet... It should probably hide by default
> or better to change look so it is clearly visible that buttons are
> disabled... 

They do show up disabled in other themes, but not in the default one, don't know why. If you pick for example "Ambiance-maverick" from the preferences, it'll work.
The default theme will have to be changed or replaced with other.


> I can not open preferences... Right click on applet -> preferences does
> nothing and if gnome-panel is restarted form terminal it also prints warning
> about invalid cast.

No idea about this. To me it works just fine and get no such warnings. Any way of knowing from which line the warning comes from?


> > > 2) window-title crash panel with schema
> > > 'org.gnome.gnome-applets.window-title' does not contain a key named
> > > 'button-minimize-hidden'. :(
> > 
> > That's because of the bug in gnome-panel you mentioned. It is fixed with:
> > > info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> > > G_MODULE_BIND_LOCAL);
> 
> No, at least I fail to see how that could be related.
> 
> Did quick grep... Key 'button-minimize-hidden' is available in
> *windows-buttons.gschema.xml not in *window-title.gschema.xml. So why
> window-title tries to access key from window-buttons applet schema? Typo /
> copy&paste error?

This is because if you install both applets together, or one after the other without restarting gnome-panel, then when accessing the window-title preferences, it tries to use the functions of the same name of the other applet, and therefore you get that error.
Changing the line you mentioned in the gnome-panel source seems to fix the issue.

Try the following:
1. Remove window-buttons applet (if you've added it) and restart gnome-panel
2. Add window-title applet
3. You'll see that everything works OK and you won't get the error you mention.
Comment 47 Alberts Muktupāvels 2017-03-27 19:55:45 UTC
(In reply to Gabriel Finkelstein from comment #46)
> (In reply to Alberts Muktupāvels from comment #45)
> > (In reply to Gabriel Finkelstein from comment #44)
> > > > 1) window-buttons does not work for me... I can add applet, but clicking on
> > > > minimize and maximize does nothing. Trying to open I get warning about
> > > > invalid cast from GtkBuilder to GtkWindow. :(
> > > 
> > > window-buttons handles only maximized windows. I you have no maximized
> > > windows then it does nothing. Perhaps you should enable the preference
> > > "Behavior" > "Hide when no active maximized window".
> > 
> > Oh, ok. I have never used that applet... It should probably hide by default
> > or better to change look so it is clearly visible that buttons are
> > disabled... 
> 
> They do show up disabled in other themes, but not in the default one, don't
> know why. If you pick for example "Ambiance-maverick" from the preferences,
> it'll work.
> The default theme will have to be changed or replaced with other.

Ok. This of course should be fixed, but not required. We should add Adwaita specific css to gnome-applets...

> > I can not open preferences... Right click on applet -> preferences does
> > nothing and if gnome-panel is restarted form terminal it also prints warning
> > about invalid cast.
> 
> No idea about this. To me it works just fine and get no such warnings. Any
> way of knowing from which line the warning comes from?

I guess same bug caused by symbol conflicts...

> > > > 2) window-title crash panel with schema
> > > > 'org.gnome.gnome-applets.window-title' does not contain a key named
> > > > 'button-minimize-hidden'. :(
> > > 
> > > That's because of the bug in gnome-panel you mentioned. It is fixed with:
> > > > info->module = g_module_open (info->location, G_MODULE_BIND_LAZY |
> > > > G_MODULE_BIND_LOCAL);
> > 
> > No, at least I fail to see how that could be related.
> > 
> > Did quick grep... Key 'button-minimize-hidden' is available in
> > *windows-buttons.gschema.xml not in *window-title.gschema.xml. So why
> > window-title tries to access key from window-buttons applet schema? Typo /
> > copy&paste error?
> 
> This is because if you install both applets together, or one after the other
> without restarting gnome-panel, then when accessing the window-title
> preferences, it tries to use the functions of the same name of the other
> applet, and therefore you get that error.
> Changing the line you mentioned in the gnome-panel source seems to fix the
> issue.
> 
> Try the following:
> 1. Remove window-buttons applet (if you've added it) and restart gnome-panel
> 2. Add window-title applet
> 3. You'll see that everything works OK and you won't get the error you
> mention.

Yes, you are right. Thanks! Pushed fix to gnome-panel master and gnome-3-24 branches.

I marked all patches as obsolete, but I have pushed your work to master and gnome-3-24 branch with few changes - changed commit message, removed old gconf schemas files and fix newline warning.

Applets still requires some work. Applets prints some runtime warnings and criticals that should be fixed.
Comment 48 Gabriel Finkelstein 2017-03-27 23:03:53 UTC
Created attachment 348844 [details] [review]
window-buttons: declare functions as static
Comment 49 Gabriel Finkelstein 2017-03-27 23:37:57 UTC
Created attachment 348848 [details] [review]
window-title: inherit fg color of theme

Instead of listening to the change-background signal which got removed, simply don't declare a color by default and inherit the one defined by the theme.
Comment 50 Alberts Muktupāvels 2017-03-28 11:25:21 UTC
Comment on attachment 348844 [details] [review]
window-buttons: declare functions as static

Pushed modified version...
Comment 51 Alberts Muktupāvels 2017-03-28 12:40:46 UTC
Comment on attachment 348848 [details] [review]
window-title: inherit fg color of theme

This can be replaced with GtkWidget style-updated signal. Only API to get color is different...

That would be short-term solution. In long-term applet should use panel_applet_add_text_class to make sure that applet text color follows panel settings.
Comment 52 Alberts Muktupāvels 2017-03-28 14:44:37 UTC
(In reply to Gabriel Finkelstein from comment #42)
> The only problem that remains is that all applets use the same "About"
> dialog, as if that dialog was stuck. (i.e. after opening the "About" dialog
> of applet X, every other applet will show the "About" dialog of applet X).
> But I think this is a gnome-panel bug, because I can reproduce it event with
> the official builds.

Ok, problem is gtk_show_about_dialog usage. Applets should not use this function to show about dialog. The problem is that that function use static variable to create about dialog only once and then reuse it.

This was not problem when applets was out-of-process, but now when all applets are in-process this causes problem...

If you want, feel free to open new bug and attach patches that replaces gtk_show_about_dialog usage with gtk_about_dialog_new + related function calls that sets up needed data/info.

Or just do something similar to:
https://git.gnome.org/browse/gtk+/tree/gtk/gtkaboutdialog.c#n2493

Not going to fix this myself because I plan to replace libpanel-applet with new libgnome-panel library where things will work differently.
Comment 53 Gabriel Finkelstein 2017-03-28 22:12:24 UTC
Created attachment 348909 [details] [review]
window-title: use text color of panel
Comment 54 Khurshid Alam 2017-03-29 05:27:22 UTC
Great work everyone. Thank you all. Waiting for it to land on Zesty.
Comment 55 Alberts Muktupāvels 2017-03-29 10:59:43 UTC
Gabriel, can you open new bug for that patch and attach it there so we can close this bug?

Khurshid, I don't think that gnome-panel and gnome-applets 3.24 will be in zesty...
Comment 56 Khurshid Alam 2017-03-29 11:35:57 UTC
Alberts, why is that? If it goes into sid before hard-freeze (on zesty), it can it still make it into zesty. No? Anyway, I can always use PPA.
Comment 57 Dmitry Shachnev 2017-03-31 12:36:24 UTC
(In reply to Khurshid Alam from comment #56)
> Alberts, why is that? If it goes into sid before hard-freeze (on zesty), it
> can it still make it into zesty. No? Anyway, I can always use PPA.

Zesty is already in hard freeze, and there are just two weeks remaining before the release:

https://lists.ubuntu.com/archives/ubuntu-devel-announce/2017-March/001207.html

My plan is to upload gnome-panel and gnome-applets 3.24 to Ubuntu after the Zesty+1 archive opens.

Debian sid is also frozen, so I can’t upload there. I will upload to experimental though.
Comment 58 Gabriel Finkelstein 2017-03-31 21:46:44 UTC
(In reply to Alberts Muktupāvels from comment #55)
> Gabriel, can you open new bug for that patch and attach it there so we can
> close this bug?

Done.
https://bugzilla.gnome.org/show_bug.cgi?id=780790