On Mon, 2008-05-19 at 01:32 +0000, [EMAIL PROTECTED] 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 <[EMAIL PROTECTED]>
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.
