GNOME Bugzilla – Bug 326272
Help/About dialog isn't HIG compliant
Last modified: 2006-02-13 21:42:55 UTC
Please describe the problem: "gucharmap" should be replace with "Character Map" in the Help/About dialog Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Good point. Behnam?
http://www.unicode.org/copyright.html#Exhibit1: """Except as contained in this notice, the name of a copyright holder shall not be used in advertising or otherwise to promote the sale, use or other dealings in these Data Files or Software without prior written authorization of the copyright holder.""" I think we MOST remove the "Unicode" character from the product name, and just use it in the info. As the program has a binary named "gnome-character-map", I suggest changing the title to "Gnome Character Map".
According to GNOME HIG http://developer.gnome.org/projects/gup/hig/2.0/menus-standard.html#menu-standard-help Help/About dialog should contain "... a pointer to the licence under which the application is made available." gucharmap application doesn't provide such a pointer. gnome-dictionary for example provides such a pointer in its Help/About dialog
Petrosyan, yes, we discussed it at bug #328775 – Adding Unicode's copyright info – and I'm going to add the "Licence" button, etc to the About dialog. Looking around, I found that the title in the About dialog is same as Product name in the bugzilla (that's cvs module name in some way). It really makes sense. I think we HAVE TO let it be "gucharmap x.y.z" (or just change it to title case) and change the description to "GNOME Character Map \n Based on Unicode Character Database 4.1". I'm going to patch these today, to get them in 1.6 release.
FYI: http://live.gnome.org/ModuleMaintenanceWorkspaces
Created attachment 58684 [details] [review] patch to clean up Help/About dialog +++ gucharmap/Makefile.am, adds unicode-version.h. +++ gucharmap/gen-guch-unicode-tables.pl, adds generate_unicode_version, adds #ifndef/#define/#endif to generated header files. +++ gucharmap/gucharmap-unicode-info.c +++ gucharmap/gucharmap-unicode-info.h, adds gucharmap_get_unicode_version(). +++ gucharmap/gucharmap-window.c, Help/About dialog: corrects name and comment; adds Unicode Copyright, link to l.g.o/Gucharmap, and License button; puts human-readable content of GPL and Unicode Copyright on the License dialog, +++ gucharmap/unicode-blocks.h, +++ gucharmap/unicode-categories.h, +++ gucharmap/unicode-names.h, +++ gucharmap/unicode-nameslist.h, +++ gucharmap/unicode-scripts.h, +++ gucharmap/unicode-unihan.h, #ifndef/#define/#endif added. +++ gucharmap/unicode-version.h new generated file, current version is 4.1.0. behdad, would you take a look asap and let me know your opinion?
Created attachment 58702 [details] [review] old patch, plus chpe's patch to Bug 328912 – misc fixes there's many common files in these two patches, so i merged them. now gucharmap needs (almost) cvs version of libgnome and libbonobo to get compiled.
Created attachment 58712 [details] [review] old patch, in lib files, changed <glib/gi18n.h> to <glib/gi18n-lib.h>. ok, it's complete at last. behdad, any opinion? are you ok with the copyright and license issues?
Split it into separate patches. There's no way for me to review this huge patch. the unicode version should be ifdef'ed, not static const. +G_CONST_RETURN gchar * +gucharmap_get_unicode_version () +{ + return _(unicode_version); +} What the heck is _() doing here? + N_("Gucharmap is free software; you can redistribute it and/or modify \n" + "it under the terms of the GNU General Public License as published by \n" + "the Free Software Foundation; either version 2 of the License, or \n" The spaces before \n should be removed. And ChangeLog entries are title-cased. You have duplicate entries in ChangeLog too... As I said, I cannot review this. 4 or 5 separate patches please.
(In reply to comment #9) > the unicode version should be ifdef'ed, not static const. > > +G_CONST_RETURN gchar * > +gucharmap_get_unicode_version () > +{ > + return _(unicode_version); > +} > > What the heck is _() doing here? gucharmap_get_unicode_version() returns a translated string of the unicode version. It's needed for the About dialog. If you have better idea about translating numbers in strings, change it. Also we can add a #define for unicode version for preprocess needs, but it's useless till we support more than one unicode version. > > + N_("Gucharmap is free software; you can redistribute it and/or modify \n" > + "it under the terms of the GNU General Public License as published by > \n" > + "the Free Software Foundation; either version 2 of the License, or \n" > > The spaces before \n should be removed. I looked at nautilus. I'm not sure if copying a multi-line text to a single-line entry, always convert 'newline's to 'space's. > > And ChangeLog entries are title-cased. You have duplicate entries in ChangeLog > too... OK, I'll correct it. > > As I said, I cannot review this. 4 or 5 separate patches please. > More than half of the patch is for generated unicode-*.h files. You can ignore them at all. Also you can review chpe's patch first, and then look at this. Thanks, BTW.
(In reply to comment #10) > (In reply to comment #9) > > the unicode version should be ifdef'ed, not static const. > > > > +G_CONST_RETURN gchar * > > +gucharmap_get_unicode_version () > > +{ > > + return _(unicode_version); > > +} > > > > What the heck is _() doing here? > > gucharmap_get_unicode_version() returns a translated string of the unicode > version. It's needed for the About dialog. If you have better idea about > translating numbers in strings, change it. Also we can add a #define for > unicode version for preprocess needs, but it's useless till we support more > than one unicode version. I'm not sure the Unicode version should be translated. No, let it be in Latin. It's not a number, it's a version string. > > + N_("Gucharmap is free software; you can redistribute it and/or modify \n" > > + "it under the terms of the GNU General Public License as published by > > \n" > > + "the Free Software Foundation; either version 2 of the License, or \n" > > > > The spaces before \n should be removed. > > I looked at nautilus. I'm not sure if copying a multi-line text to a > single-line entry, always convert 'newline's to 'space's. Either test it and make sure (I'm pretty sure Pango and Gtk+ do the right thing), or remove the '\n's. > > As I said, I cannot review this. 4 or 5 separate patches please. > > More than half of the patch is for generated unicode-*.h files. You can ignore > them at all. Ok, then put them in a separate patch such that I know. But I do want to check them too. Just that the level of attention to details needed to check them varies. > Also you can review chpe's patch first, and then look at this. > Thanks, BTW. I have reviewed chpe's patch, and it needed work. Now you have mashed that into a huge patch. Please submit a reworked version of his patch in his bug, and if you like, submit incremental patches based on that here. AFAIU, you simply replaced gi18n.h with gi18n-lib.h in his patch. That doesn't work, since we have no place to initalize gettext. One way to do this is to do it in the class initialization and simply use _() and gi18n-lib.h in files that implement the class, and use Noah's currect workaround in the single file that uses _() but isn't implementation of the class.
Btw, you can remove the \n's if/when you depend on gtk 2.8, and use 'wrap-license' = TRUE for the about dialogue.
Created attachment 59260 [details] [review] patch for source files.
Created attachment 59261 [details] [review] patch for the generated header files.
Committed after s/Unicode data/Unicode data files/ 2006-02-13 Behdad Esfahbod <behdad@gnome.org> Bug 326272 – Help/About dialog isn't HIG compliant Patch from Behnam "ZWNJ" Esfahbod * gucharmap/gucharmap-window.c (help_about): Use "GNOME Character Map" instead of "gucharmap". Add license information. * gucharmap/gen-guch-unicode-tables.pl: * gucharmap/unicode-blocks.h: * gucharmap/unicode-categories.h: * gucharmap/unicode-names.h: * gucharmap/unicode-nameslist.h: * gucharmap/unicode-scripts.h: * gucharmap/unicode-unihan.h: Guard headers from multiple inclusion.