GNOME Bugzilla – Bug 513438
Contact Editor - Evolution allows entering not valid data in the e-mail field
Last modified: 2012-06-16 22:22:15 UTC
BNC bug https://bugzilla.novell.com/show_bug.cgi?id=171392 Contacts - Contact Editor - Evolution allows entering not valid data in the e-mail and telephone fields.
too vague, please elaborate and post more info here.
Created attachment 104198 [details] Screen shot of contact with wrong data under red marks
When you create a contact,email and telephone fields are there.If you enter anything which is not relevant to respective field ,it won't give you any warning. Like in telephone field,if you enter strings instead of numbers,it will save information. In email field it will save data without any valid email id.
Bumping version to a stable release.
Created attachment 113891 [details] [review] i have added some checks to validate phone numbers and email ids
Roshan, i will try your patch soon but i guess you haven't included translation comment for new strings. Please see http://live.gnome.org/TranslationProject/DevGuidelines/Use comments
It's not missing comments, it's missing translation AT ALL.
Created attachment 113909 [details] [review] I think it is okay now just check ..
Comment on attachment 113909 [details] [review] I think it is okay now just check .. >Index: addressbook/gui/contact-editor/e-contact-editor.c >=================================================================== >--- addressbook/gui/contact-editor/e-contact-editor.c (revision 35717) >+++ addressbook/gui/contact-editor/e-contact-editor.c (working copy) >@@ -31,6 +31,8 @@ > #include <libgnomeui/gnome-window-icon.h> > #include <libgnome/gnome-util.h> > #include <glib/gi18n.h> >+#include <glib.h> >+ > #include <libgnome/gnome-help.h> > > #include <gdk-pixbuf/gdk-pixbuf.h> >@@ -3075,17 +3077,78 @@ > > } > >+/* This function validates a phone entry */ >+static gboolean >+is_valid_phone(EABEditor *editor) >+{ >+ gint i; >+ const gchar *valid_entry = "1234567890+#*" ; >+ gchar *validated_phone; >+ >+ for (i = 1; i <= PHONE_SLOTS; i++) { >+ gchar *phone; >+ gint phone_type; >+ >+ extract_phone_record (E_CONTACT_EDITOR(editor), i, &phone, &phone_type); >+ validated_phone = g_strdup(phone); >+ if (!STRING_IS_EMPTY (phone)) { >+ g_strcanon (validated_phone, valid_entry, '?'); >+ if (!(g_str_equal (phone, validated_phone))) >+ { >+ return TRUE; >+ } >+ } >+ } >+ return FALSE; >+} > >+/* This function validates an email entry */ >+static gboolean >+is_valid_email(EABEditor *editor) >+{ >+ gint i; >+ gchar* pos_of_at, *pos_of_dot; >+ gssize len; >+ >+ for (i = 1; i <= EMAIL_SLOTS; i++) { >+ gchar *address; >+ gint location; >+ >+ extract_email_record (E_CONTACT_EDITOR(editor), i, &address, &location); >+ >+ if (!STRING_IS_EMPTY (address)) { >+ pos_of_at = g_strrstr (address, "@"); >+ pos_of_dot = g_strrstr (address, "."); >+ if (pos_of_at) { >+ len = pos_of_at - address; >+ if (g_strrstr_len (address, len, "@")) >+ return TRUE; >+ // Check if "." proceeds "@" >+ len = pos_of_at - pos_of_dot; >+ if (len > 0) >+ return TRUE; >+ // Check if "@" is the first character or "." is the last character >+ if (g_str_has_prefix (address,"@") || g_str_has_suffix(address,".")) >+ return TRUE; >+ } >+ else >+ return TRUE; >+ } >+ } >+ return FALSE; >+} >+ > /* insert checks here (date format, for instance, etc.) */ > static gboolean > e_contact_editor_is_valid (EABEditor *editor) > { > EContactEditor *ce = E_CONTACT_EDITOR (editor); > GtkWidget *widget; >+ GtkWidget *msg_dialog; > gboolean validation_error = FALSE; > EIterator *iter; > GString *errmsg = g_string_new (_("The contact data is invalid:\n\n")); >- >+ > widget = glade_xml_get_widget (ce->gui, "dateedit-birthday"); > if (!(e_date_edit_date_is_valid (E_DATE_EDIT (widget)))) { > g_string_append_printf (errmsg, _("'%s' has an invalid format"), >@@ -3100,7 +3163,30 @@ > e_contact_pretty_name (E_CONTACT_ANNIVERSARY)); > validation_error = TRUE; > } >- >+ >+ if(is_valid_email(editor)) >+ { msg_dialog = gtk_message_dialog_new(NULL, >+ GTK_DIALOG_MODAL, >+ GTK_MESSAGE_WARNING, >+ GTK_BUTTONS_OK, >+ "%s", _("Invalid email")); >+ gtk_dialog_run (GTK_DIALOG(msg_dialog)); >+ gtk_widget_destroy (msg_dialog); >+ return FALSE; >+ } >+ >+ if (is_valid_phone(editor)) >+ { >+ msg_dialog = gtk_message_dialog_new(NULL, >+ GTK_DIALOG_MODAL, >+ GTK_MESSAGE_WARNING, >+ GTK_BUTTONS_OK, >+ "%s", _("Invalid phone number")); >+ gtk_dialog_run (GTK_DIALOG(msg_dialog)); >+ gtk_widget_destroy (msg_dialog); >+ return FALSE; >+ } >+ > iter = e_list_get_iterator (ce->required_fields); > for (e_iterator_last (iter); > e_iterator_is_valid (iter); >Index: addressbook/ChangeLog >=================================================================== >--- addressbook/ChangeLog (revision 35717) >+++ addressbook/ChangeLog (working copy) >@@ -1,3 +1,10 @@ >+2008-07-03 Roshan Kumar Singh <roshan.singh08@yahoo.com> >+ >+ ** Fix for bug #513438 >+ >+ * e-contact-editor.c: >+ Added checks for validating email and phone number. >+ > 2008-07-03 Milan Crha <mcrha@redhat.com> > > ** Fix for bug #540152
Sorry, it happened by mistake.
"Invalid email" is misleading. Think of the translation in other languages - would you understand the error message if it was translated to "Invalid message" for example?
> "Invalid email" is misleading. Okay i understood, so should I change it to "Invalid email id". I dont have much idea about translation. Can you suggest a possible error message?
and as a normal average user, you will loooove the term ID (correct spelling, please). can you please also change "phone number" to "phone ID" to make it less understandable? ;-)))
Created attachment 113993 [details] [review] Reworked the earlier patch I have made the changes as you have suggested :) . Hope it should be fine now.
Great, you really made it less understandable for a normal user and ignored any smileys that could have shown some irony. I'm kind of impressed. So why don't I write explicitly the changes I want here? Because I want developers to think from an average user point of view. Because I want developers to use language and wording that a normal person can understand, without having a degree in CS. Because I want developers to think about potential problems in translations to other languages, and translators do NOT know about the context of your string, they ONLY see the string, not the surrounding code. Last hint: When *I* meet people, I normally ask them for their email address, or for their phone number. do you REALLY ask non techy folks for their "email id"? I'd really love to know, maybe I should switch to another culture or planet if that's actually the case. At least I wouldn't even know what you speak about. Good luck.
Created attachment 113997 [details] [review] now i understood what you wanted me to do actually different people have different tastes i like to use email ID and in case if a person is using a mail client i think he will understand what "email ID" is.. also i dont have much idea about translations so i tried to follow whatever you mentioned. Please be straight forward as you were this time. I am only a beginner. Thank you for taking time to elaborate yourself !
Patch rejected. As Andre pointed out, the strings just suck. FWIW, unlike comment 13, this is not irony. Though the latest patch changes are quite comical... > + const gchar *valid_entry = "1234567890+#*" ; This is wrong. It is quite common, to denote a specific target in companies with a hyphen between the company number prefix and the direct dial. Also, it is common to use spaces to enhance readability, for example after the area-prefix. At least in the USA and other countries, company numbers and hotlines in particular do contain chars A-Z. Yes, these chars actually are on the number pad. Just some counter examples, that came to mind immediately. There may be even more valid chars in other countries commonly used. Also, the code lacks any migration. What about numbers that have been entered before? The attempt at verifying an email address is just weird. I didn't really understand what you are trying to accomplish with these tests. Seriously. + if (g_strrstr_len (address, len, "@")) Returns always true at that point. Useless. Also, if the address contains an @, all your further tests will be skipped. + len = pos_of_at - pos_of_dot; + if (len > 0) So you say the address must be valid, if there is no dot after the @, but a dot before the @... Btw, user@localhost (hint, no dot) is a bloody valid email address. As can even be the plain 'user', without any host part. The entire logic is just utterly wrong. Did you even bother to compile the code? You can't possibly have tested it... I hereby reject the next patch in advance. Yes, I am serious. Also, given both my remarks about valid chars in phone numbers and how email addresses may be used, I seriously question the use of any such functionality, and I am inclined to close this bug WONTFIX. Is this really a bug?
http://en.wikipedia.org/wiki/Phonewords is interesting. In Germany it's popular to write "(01234) 56789".
(In reply to comment #16) > in case if a person is using a mail client i think he will understand what > "email ID" is.. my girlfriend would not. just tried.
another case: if you call an employee directly in a bigger company or administration it's popular to use a -. e.g. switchboard has 0 and an employee has 123, it's like (01234) 567-0 and (01234) 567-123. and it's also popular to write 01234 / 567-0 instead. GNOME, the desktop of the interface nazis, "fixes" that actually remove capabilities that the user had before. Here we go! http://mail.gnome.org/archives/usability/2005-December/msg00022.html
Right, the example I meant to mention, but forgot. Parenthesis are common in Germany for area-prefix. (I mentioned the dash already, though. ;) Rejecting the part about phone numbers WONTFIX. Regarding email addresses, there are just a very few chars that are not allowed. And even these MIGHT be allowed in some corporate environment using local email addresses, that are not actually routed through the net as is. The space (and any non printable chars) *probably* would be a somewhat sane and safe blacklist. Of course, not allowing any non-ASCII, 8-bit chars. This one is up for discussion. However, don't even bother to attach any new patch, before coming up with a solid proposal, how a valid address should look like. That includes reasoning. (Yes, first define what a valid address is. Take corporate envs into account. Then throw a patch at bugzilla.)
I guess you are quite right, there are vast differences how phone numbers are used in different countries and same is for email address. No more patches from me now onwards.
*** Bug 663373 has been marked as a duplicate of this bug. ***
*** Bug 663374 has been marked as a duplicate of this bug. ***
Implementation comments in bug 663374 comment 5
*** Bug 213724 has been marked as a duplicate of this bug. ***
I have no idea why this is still open - resolving as WONTFIX.