r4177 mikeL - in /branches/mikeL/netsurf/gtk: dialogs/gtk_about.c gtk_scaffolding.c res/license res/netsurf.glade

Rob Kendrick rjek at netsurf-browser.org
Mon May 19 09:44:52 BST 2008


On Mon, 2008-05-19 at 01:32 +0000, netsurf at semichrome.net wrote:
> Author: mikeL
> Date: Sun May 18 20:32:21 2008
> New Revision: 4177
> 
> URL: http://source.netsurf-browser.org?rev=4177&view=rev
> Log:
> Redesigned about dialog as a GtkAboutDialog and removed the respective section from netsurf.glade (May need string revision)

I have some concerns with this commit;
  * Artwork/Translations are duplicated.  This is fine as long as we
    still have the information on who did what.
  * The one-per-line listing of the people involved feels wasteful, as
    half the window is empty.
  * It doesn't use the whole NetSurf logo, just the icon.
  * The licence text is not in a fixed-pitch font, and it contains
    preformatted sections, so looks odd/wrong in places.

Other concerns are in-line.

> Added: branches/mikeL/netsurf/gtk/dialogs/gtk_about.c
> URL: http://source.netsurf-browser.org/branches/mikeL/netsurf/gtk/dialogs/gtk_about.c?rev=4177&view=auto
> ==============================================================================
> --- branches/mikeL/netsurf/gtk/dialogs/gtk_about.c (added)
> +++ branches/mikeL/netsurf/gtk/dialogs/gtk_about.c Sun May 18 20:32:21 2008
> @@ -1,0 +1,51 @@
> +/*
> + * Copyright 2008 Rob Kendrick <rjek at rjek.com>

This file is nothing to do with me :)  You should put your own name and
email address here.

> + * This file is part of NetSurf, http://www.netsurf-browser.org/
> + *
> + * NetSurf is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * NetSurf is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "gtk/gtk_gui.h"
> +#include "desktop/browser.h"
> +
> +GtkAboutDialog* about_dialog;
> +
> +gchar *authors[] = {"Kevin Bagust", "John M Bell", "James Bursa", "Matthew Hambley", "Rob Jackson", "Rob Kendrick", "Jeffrey Lee", "Adrian Lees", "Phil Mellor", "Philip Pemberton", "Daniel Silverstone", "Andrew Timmins", "John Tytgat", "Richard Wilson", NULL};
> +gchar *translators = "Sebastian Barthel \nBruno D'Arcangeli \nMichael Drake \nAndrew Duffell \nRichard Hallas \nGerard van Katwijk \nJérôme Mathevet \nSimon Voortman.";
> +gchar *artists[] = {"Sebastian Barthel", "Bruno D'Arcangeli", "Michael Drake", "Andrew Duffell", "Richard Hallas", "Gerard van Katwijk", "Jérôme Mathevet", "Simon Voortman", NULL};

Why are the authors and artists listed as seperate strings, but the
translators are one string?  Is this just an uglyness in the GTK API?

> +gchar *name = "NetSurf";
> +gchar *description = "Small as a mouse, fast as a cheetah, and available for free.\nNetsurf is a web browser for RISC OS and UNIX-like platforms.";

"NetSurf" has a capital S.

> +gchar *url = "http://www.netsurf-browser.org";
> +gchar *url_label = "NetSurf Website";
> +gchar *copyright = "Copyright © 2003 - 2008 The NetSurf Developers";
> +
> +gchar* license = "license"; 

"Licence" as a noun is spelt with a C, not an S.  American English
doesn't have this distinction (they are interchangeable), but they do in
British English.  (Think devise/device and advise/advice when you spell
license/licence - it's the same rule.)

Could all these variables be static?  I don't think they need to be
accessed outside this link unit, and the pollute the namespace.

> +static void launch_url (GtkAboutDialog *about_dialog, const gchar *url, gpointer data){
> +      struct browser_window *bw = data;
> +	 browser_window_go(bw, url, 0, true);
> + }
> +
> +void nsgtk_about_dialog_init(GtkWindow *parent, struct browser_window *bw, char *version) {
> +	g_file_get_contents(g_strconcat(res_dir_location, license, NULL), &license, NULL, NULL);
> +	gtk_about_dialog_set_url_hook (launch_url, (gpointer) bw, NULL);
> +	
> +	gtk_show_about_dialog(parent, "artists", artists, "authors", authors,
> +	"comments", description,"copyright", copyright, "license", license,
> +	"program-name", name, "translator-credits", translators,
> +	"version", version, "website", url, "website-label", url_label, 
> +	"wrap-license", FALSE, NULL);
> +} 
> +	
> 
> Modified: branches/mikeL/netsurf/gtk/gtk_scaffolding.c
> URL: http://source.netsurf-browser.org/branches/mikeL/netsurf/gtk/gtk_scaffolding.c?rev=4177&r1=4176&r2=4177&view=diff
> ==============================================================================
> --- branches/mikeL/netsurf/gtk/gtk_scaffolding.c (original)
> +++ branches/mikeL/netsurf/gtk/gtk_scaffolding.c Sun May 18 20:32:21 2008
> @@ -35,6 +35,7 @@
>  #include "gtk/gtk_plotters.h"
>  #include "gtk/gtk_scaffolding.h"
>  #include "gtk/dialogs/gtk_options.h"
> +#include "gtk/dialogs/gtk_about.c"
>  #include "gtk/gtk_completion.h"
>  #include "gtk/gtk_throbber.h"
>  #include "gtk/gtk_history.h"
> @@ -666,8 +667,8 @@
>  
>  MENUHANDLER(about)
>  {
> -	gtk_widget_show(GTK_WIDGET(wndAbout));
> -	gdk_window_raise(GTK_WIDGET(wndAbout)->window);
> +	struct gtk_scaffolding *gw = (struct gtk_scaffolding *)g;
> +	nsgtk_about_dialog_init(gw->window, nsgtk_get_browser_for_gui(gw->top_level),netsurf_version);
>  	return TRUE;
>  }
>  
> 
> Added: branches/mikeL/netsurf/gtk/res/license
> URL: http://source.netsurf-browser.org/branches/mikeL/netsurf/gtk/res/license?rev=4177&view=auto
> ==============================================================================
> --- branches/mikeL/netsurf/gtk/res/license (added)
> +++ branches/mikeL/netsurf/gtk/res/license Sun May 18 20:32:21 2008

Could this be a symlink to the COPYING file instead, perhaps?

B.




More information about the netsurf-dev mailing list