Merge into trunk : misc-fixes : Code : The Me Menu

Status:	Merged
Merge reported by:	David Barth
Merged at revision:	not available
Proposed branch:	lp:~dbarth/indicator-me/misc-fixes
Merge into:	lp:indicator-me
Diff against target:	0 lines
To merge this branch:	bzr merge lp:~dbarth/indicator-me/misc-fixes
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Ted Gould (community)		2010-03-23	Approve on 2010-03-23
Review via email: mp+21967@code.launchpad.net

Description of the change

Misc fixes:
- if visible, always enable the broadcast field, whenever gwibber is running or not
- missing fallback icon for the user-indeterminate state
- fix typo in gwibber-service and more checks for parameters in the entry activate callback
- fix some potential crashers and get rid of some spurious error message

Revision history for this message

Ted Gould (ted) wrote on 2010-03-23:

#

Download full text (12.8 KiB)

      * I think that entry_activate_cb() shouldn't use the parameter
        name "gmi" as other places that's be used for "GTK Menu item"
        and might confuse things. (it confused me in the limited scope
        of the diff)
      * I'm unsure why the static was removed from
        menu_visibility_changed.
      * In me-service-gwibber.c where service_proxy was changed to
        accounts_proxy the error comment should also be changed.

review approve

On Tue, 2010-03-23 at 18:42 +0000, David Barth wrote:
> David Barth has proposed merging lp:~dbarth/indicator-me/misc-fixes into lp:indicator-me.
>
> Requested reviews:
> Indicator Applet Developers (indicator-applet-developers)
>
>
> Misc fixes:
> - if visible, always enable the broadcast field, whenever gwibber is running or not
> - missing fallback icon for the user-indeterminate state
> - fix typo in gwibber-service and more checks for parameters in the entry activate callback
> - fix some potential crashers and get rid of some spurious error message
> differences between files attachment (review-diff.txt)
> === modified file 'data/icons/16x16/status/Makefile.am'
> --- data/icons/16x16/status/Makefile.am 2009-09-16 20:59:37 +0000
> +++ data/icons/16x16/status/Makefile.am 2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
> iconsdir = $(INDICATORICONSDIR)/hicolor/16x16/status
>
> icons_DATA = \
> + user-indeterminate.png \
> user-available.png \
> user-offline..png \
> user-away.png \
>
> === added file 'data/icons/16x16/status/user-indeterminate.png'
> Binary files data/icons/16x16/status/user-indeterminate.png 1970-01-01 00:00:00 +0000 and data/icons/16x16/status/user-indeterminate.png 2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/22x22/status/Makefile.am'
> --- data/icons/22x22/status/Makefile.am 2009-09-16 20:59:37 +0000
> +++ data/icons/22x22/status/Makefile.am 2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
> iconsdir = $(INDICATORICONSDIR)/hicolor/22x22/status
>
> icons_DATA = \
> + user-indeterminate..png \
> user-available.png \
> user-offline.png \
> user-away.png \
>
> === added file 'data/icons/22x22/status/user-indeterminate.png'
> Binary files data/icons/22x22/status/user-indeterminate.png 1970-01-01 00:00:00 +0000 and data/icons/22x22/status/user-indeterminate.png 2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/24x24/status/Makefile.am'
> --- data/icons/24x24/status/Makefile.am 2009-09-16 20:59:37 +0000
> +++ data/icons/24x24/status/Makefile.am 2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
> iconsdir = $(INDICATORICONSDIR)/hicolor/24x24/status
>
> icons_DATA = \
> + user-indeterminate.png \
> user-available.png \
> user-offline.png \
> user-away.png \
>
> === added file 'data/icons/24x24/status/user-indeterminate.png'
> Binary files data/icons/24x24/status/user-indeterminate.png 1970-01-01 00:00:00 +0000 and data/icons/24x24/status/user-indeterminate.png 2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/32x32/status/Makefile.am'
> --- data/icons/32x32/status/Makefile.am 2009-09-16 20:59:37 +0000
> +++ data/icons/32x32/status/Makefile.am 2010-03-23 18:42:17 +0000
> @@ -...

* I think that entry_activate_cb() shouldn't use the parameter
        name "gmi" as other places that's be used for "GTK Menu item"
        and might confuse things.  (it confused me in the limited scope
        of the diff)
      * I'm unsure why the static was removed from
        menu_visibility_changed.
      * In me-service-gwibber.c where service_proxy was changed to
        accounts_proxy the error comment should also be changed.

review approve

On Tue, 2010-03-23 at 18:42 +0000, David Barth wrote:
> David Barth has proposed merging lp:~dbarth/indicator-me/misc-fixes into lp:indicator-me.
> 
> Requested reviews:
>   Indicator Applet Developers (indicator-applet-developers)
> 
> 
> Misc fixes:
> - if visible, always enable the broadcast field, whenever gwibber is running or not
> - missing fallback icon for the user-indeterminate state
> - fix typo in gwibber-service and more checks for parameters in the entry activate callback
> - fix some potential crashers and get rid of some spurious error message
> differences between files attachment (review-diff.txt)
> === modified file 'data/icons/16x16/status/Makefile.am'
> --- data/icons/16x16/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/16x16/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/16x16/status
>  
>  icons_DATA =  \
> +	user-indeterminate.png			\
>  	user-available.png			\
>  	user-offline..png		\
>  	user-away.png			\
> 
> === added file 'data/icons/16x16/status/user-indeterminate.png'
> Binary files data/icons/16x16/status/user-indeterminate.png	1970-01-01 00:00:00 +0000 and data/icons/16x16/status/user-indeterminate.png	2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/22x22/status/Makefile.am'
> --- data/icons/22x22/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/22x22/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/22x22/status
>  
>  icons_DATA =  \
> +	user-indeterminate..png			\
>  	user-available.png			\
>  	user-offline.png		\
>  	user-away.png			\
> 
> === added file 'data/icons/22x22/status/user-indeterminate.png'
> Binary files data/icons/22x22/status/user-indeterminate.png	1970-01-01 00:00:00 +0000 and data/icons/22x22/status/user-indeterminate.png	2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/24x24/status/Makefile.am'
> --- data/icons/24x24/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/24x24/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/24x24/status
>  
>  icons_DATA =  \
> +	user-indeterminate.png			\
>  	user-available.png			\
>  	user-offline.png		\
>  	user-away.png			\
> 
> === added file 'data/icons/24x24/status/user-indeterminate.png'
> Binary files data/icons/24x24/status/user-indeterminate.png	1970-01-01 00:00:00 +0000 and data/icons/24x24/status/user-indeterminate.png	2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/32x32/status/Makefile.am'
> --- data/icons/32x32/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/32x32/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/32x32/status
>  
>  icons_DATA =  \
> +	user-indeterminate.png			\
>  	user-available.png			\
>  	user-offline.png		\
>  	user-away.png			\
> 
> === added file 'data/icons/32x32/status/user-indeterminate.png'
> Binary files data/icons/32x32/status/user-indeterminate.png	1970-01-01 00:00:00 +0000 and data/icons/32x32/status/user-indeterminate.png	2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/48x48/status/Makefile.am'
> --- data/icons/48x48/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/48x48/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/48x48/status
>  
>  icons_DATA =  \
> +	user-indeterminate.png			\
>  	user-available.png			\
>  	user-offline.png		\
>  	user-away.png			\
> 
> === added file 'data/icons/48x48/status/user-indeterminate.png'
> Binary files data/icons/48x48/status/user-indeterminate.png	1970-01-01 00:00:00 +0000 and data/icons/48x48/status/user-indeterminate.png	2010-03-23 18:42:17 +0000 differ
> === modified file 'data/icons/scalable/status/Makefile.am'
> --- data/icons/scalable/status/Makefile.am	2009-09-16 20:59:37 +0000
> +++ data/icons/scalable/status/Makefile.am	2010-03-23 18:42:17 +0000
> @@ -2,6 +2,7 @@
>  iconsdir = $(INDICATORICONSDIR)/hicolor/scalable/status
>  
>  icons_DATA =  \
> +	user-indeterminate.svg			\
>  	user-available.svg			\
>  	user-offline.svg		\
>  	user-away.svg			\
> 
> === added file 'data/icons/scalable/status/user-indeterminate.svg'
> --- data/icons/scalable/status/user-indeterminate.svg	1970-01-01 00:00:00 +0000
> +++ data/icons/scalable/status/user-indeterminate.svg	2010-03-23 18:42:17 +0000
> @@ -0,0 +1,129 @@
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> +
> +
> +<svg
> +   xmlns:dc="http://purl.org/dc/elements/1.1/"
> +   xmlns:cc="http://creativecommons.org/ns#"
> +   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
> +   xmlns:svg="http://www.w3.org/2000/svg"
> +   xmlns="http://www.w3.org/2000/svg"
> +   xmlns:xlink="http://www.w3.org/1999/xlink"
> +   xmlns:sodipodi="http://sodipodi..sourceforge.net/DTD/sodipodi-0.dtd"
> +   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
> +   width="16px"
> +   height="16px"
> +   id="svg4908"
> +   sodipodi:version="0.32"
> +   inkscape:version="0.47 r22583"
> +   sodipodi:docname="user-available.svg"
> +   inkscape:output_extension="org.inkscape.output.svg.inkscape"
> +   inkscape:export-filename="/home/andreas/project/gossip/16x16/status-avaible.png"
> +   inkscape:export-xdpi="90"
> +   inkscape:export-ydpi="90"
> +   sodipodi:modified="true"
> +   version="1.1">
> +  <defs
> +     id="defs4910">
> +    <inkscape:perspective
> +       sodipodi:type="inkscape:persp3d"
> +       inkscape:vp_x="0 : 8 : 1"
> +       inkscape:vp_y="0 : 1000 : 0"
> +       inkscape:vp_z="16 : 8 : 1"
> +       inkscape:persp3d-origin="8 : 5.3333333 : 1"
> +       id="perspective13" />
> +    <linearGradient
> +       inkscape:collect="always"
> +       id="linearGradient5899">
> +      <stop
> +         style="stop-color:#7fe719;stop-opacity:1"
> +         offset="0"
> +         id="stop5901" />
> +      <stop
> +         style="stop-color:#67bc13;stop-opacity:1"
> +         offset="1"
> +         id="stop5903" />
> +    </linearGradient>
> +    <radialGradient
> +       inkscape:collect="always"
> +       xlink:href="#linearGradient5899"
> +       id="radialGradient5905"
> +       cx="9..466115"
> +       cy="8.9839392"
> +       fx="9.466115"
> +       fy="8.9839392"
> +       r="6.7474474"
> +       gradientTransform="matrix(1.1671849,-3.2679277e-3,2.7524467e-3,1.0150183,-1.6222549,-0.2024225)"
> +       gradientUnits="userSpaceOnUse" />
> +    <filter
> +       id="filter2819"
> +       inkscape:label="Desaturate"
> +       x="0"
> +       y="0"
> +       width="1"
> +       height="1"
> +       inkscape:menu="Color"
> +       inkscape:menu-tooltip="Render in shades of gray by reducing saturation to zero"
> +       color-interpolation-filters="sRGB">
> +      <feColorMatrix
> +         id="feColorMatrix2821"
> +         type="saturate"
> +         values="0" />
> +    </filter>
> +  </defs>
> +  <sodipodi:namedview
> +     id="base"
> +     pagecolor="#ffffff"
> +     bordercolor="#666666"
> +     borderopacity="1.0"
> +     inkscape:pageopacity="0.0"
> +     inkscape:pageshadow="2"
> +     inkscape:zoom="22.197802"
> +     inkscape:cx="14.452639"
> +     inkscape:cy="6.4458104"
> +     inkscape:current-layer="layer1"
> +     showgrid="false"
> +     inkscape:grid-bbox="true"
> +     inkscape:document-units="px"
> +     inkscape:window-width="1398"
> +     inkscape:window-height="969"
> +     inkscape:window-x="0"
> +     inkscape:window-y="26"
> +     inkscape:window-maximized="0" />
> +  <metadata
> +     id="metadata4913">
> +    <rdf:RDF>
> +      <cc:Work
> +         rdf:about="">
> +        <dc:format>image/svg+xml</dc:format>
> +        <dc:type
> +           rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
> +        <dc:title></dc:title>
> +      </cc:Work>
> +    </rdf:RDF>
> +  </metadata>
> +  <g
> +     id="layer1"
> +     inkscape:label="Layer 1"
> +     inkscape:groupmode="layer">
> +    <path
> +       sodipodi:type="arc"
> +       style="opacity:1;fill:url(#radialGradient5905);fill-opacity:1;stroke:#376e01;stroke-width:0.97113216000000002;stroke-linecap:round;stroke-linejoin:round;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:1;filter:url(#filter2819)"
> +       id="path4926"
> +       sodipodi:cx="7.9737625"
> +       sodipodi:cy="7.7221532"
> +       sodipodi:rx="6.2618814"
> +       sodipodi:ry="6.3632426"
> +       d="M 14.235644 7.7221532 A 6.2618814 6.3632426 0 1 1  1.7118812,7.7221532 A 6.2618814 6.3632426 0 1 1  14.235644 7.7221532 z"
> +       transform="matrix(1.0380267,0,0,1.0214918,-0.2769783,0.1118841)" />
> +    <path
> +       sodipodi:type="arc"
> +       style="opacity:0.4;fill:none;fill-opacity:1;stroke:#ffffff;stroke-width:1.14770162;stroke-linecap:round;stroke-linejoin:round;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:1"
> +       id="path5897"
> +       sodipodi:cx="7.9737625"
> +       sodipodi:cy="7.7221532"
> +       sodipodi:rx="6.2618814"
> +       sodipodi:ry="6.3632426"
> +       d="M 14..235644 7.7221532 A 6.2618814 6.3632426 0 1 1  1.7118812,7.7221532 A 6.2618814 6.3632426 0 1 1  14.235644 7.7221532 z"
> +       transform="matrix(0..8783303,0,0,0.8643392,0.996403,1.3254404)" />
> +  </g>
> +</svg>
> 
> === modified file 'src/indicator-me.c'
> --- src/indicator-me.c	2010-03-16 11:26:04 +0000
> +++ src/indicator-me.c	2010-03-23 18:42:17 +0000
> @@ -214,6 +214,8 @@
>  				g_error_free(error);
>  			}
>  
> +      if (status_proxy == NULL) return;
> +
>  			dbus_g_proxy_add_signal(status_proxy, "StatusIconsChanged", G_TYPE_STRING, G_TYPE_INVALID);
>  			dbus_g_proxy_connect_signal(status_proxy, "StatusIconsChanged", G_CALLBACK(status_icon_changed), NULL, NULL);
>  		}
> @@ -242,6 +244,9 @@
>  static void
>  entry_activate_cb (GtkEntry *entry, DbusmenuMenuitem *gmi)
>  {
> +  g_return_if_fail (GTK_IS_ENTRY (entry));
> +  g_return_if_fail (DBUSMENU_IS_MENUITEM (gmi));
> +
>  	GValue value = { 0 };
>  	g_value_init (&value, G_TYPE_STRING);
>  	g_value_set_static_string (&value, gtk_entry_get_text (entry));
> @@ -251,11 +256,13 @@
>  	dbusmenu_menuitem_handle_event (gmi, "send", &value, gtk_get_current_event_time());
>  }
>  
> -static gboolean
> +/* static */ gboolean
>  menu_visibility_changed (GtkWidget          *widget,
>                           IdoEntryMenuItem   *menuitem)
>  {
> -  gtk_menu_shell_select_item (GTK_MENU_SHELL (widget), GTK_WIDGET (menuitem));
> +  if (GTK_IS_WIDGET (widget)
> +      && IDO_IS_ENTRY_MENU_ITEM (menuitem))
> +    gtk_menu_shell_select_item (GTK_MENU_SHELL (widget), GTK_WIDGET (menuitem));
>  
>    return FALSE;
>  }
> @@ -298,7 +305,8 @@
>  	/* disconnect the activate signal that newitem_base connected with the wrong
>  	   widget, ie menuitem, and re-connect it with the /entry/ instead */
>  	gulong signal_id = g_signal_handler_find (GTK_MENU_ITEM (ido), G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, parent);
> -	g_signal_handler_disconnect(GTK_MENU_ITEM (ido), signal_id);
> +  if (signal_id > 0)
> +    g_signal_handler_disconnect(GTK_MENU_ITEM (ido), signal_id);
>  
>  	g_signal_connect (DBUSMENU_MENUITEM (newitem), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK (entry_prop_change_cb), entry);
>  	g_signal_connect (GTK_ENTRY (entry), "activate", G_CALLBACK (entry_activate_cb), newitem);
> 
> === modified file 'src/me-service-gwibber.c'
> --- src/me-service-gwibber.c	2010-03-17 16:19:44 +0000
> +++ src/me-service-gwibber.c	2010-03-23 18:42:17 +0000
> @@ -297,7 +297,7 @@
>  									  DBUS_SERVICE_ADDRESS,
>  									  DBUS_SERVICE_ACCOUNTS_OBJECT,
>  									  DBUS_SERVICE_ACCOUNTS_INTERFACE);
> -		if (priv->service_proxy == NULL) {
> +		if (priv->accounts_proxy == NULL) {
>  			g_warning ("can't setup service_proxy");
>  			return;
>  		}
> 
> === modified file 'src/me-service.c'
> --- src/me-service.c	2010-03-16 20:42:59 +0000
> +++ src/me-service.c	2010-03-23 18:42:17 +0000
> @@ -188,6 +188,7 @@
>    if (! dbusmenu_menuitem_property_get_bool (broadcast_field, DBUSMENU_MENUITEM_PROP_VISIBLE))
>      dbusmenu_menuitem_property_set_bool (broadcast_field, DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE);
>  
> +#if 0
>    if (status == GWIBBER_SERVICE_STATUS_RUNNING) {
>      g_debug ("enabling the broadcast field");
>      dbusmenu_menuitem_property_set_bool (broadcast_field, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
> @@ -195,6 +196,7 @@
>      g_debug ("disabling the broadcast field");
>      dbusmenu_menuitem_property_set_bool (broadcast_field, DBUSMENU_MENUITEM_PROP_ENABLED, FALSE);
>    }
> +#endif
>  
>    return;
>  }
>

review: Approve

Revision history for this message

David Barth (dbarth) wrote on 2010-03-24:

#

Thanks, corrected in the new commit.

lp:~dbarth/indicator-me/misc-fixes updated on 2010-03-24

93. By David Barth on 2010-03-24: style & typo fixes ted pointed out in his review

The Me Menu

Merge lp:~dbarth/indicator-me/misc-fixes into lp:indicator-me

Commit message

Description of the change

Preview Diff

Subscribers