After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705338 - OOO message not spell-checked
OOO message not spell-checked
Status: RESOLVED FIXED
Product: evolution-ews
Classification: Other
Component: Account Setup Plugin
3.8.x
Other Linux
: Normal enhancement
: ---
Assigned To: Evolution EWS maintainer(s)
Evolution EWS maintainer(s)
Depends on: 329616
Blocks: 703515
 
 
Reported: 2013-08-02 09:36 UTC by David Woodhouse
Modified: 2013-11-29 12:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch adding gtkspell support allowing spell checking for OOO text views (3.04 KB, patch)
2013-11-19 14:54 UTC, Jan-Michael Brummer
reviewed Details | Review
Patch adding gtkspell support allowing spell checking for OOO text views - v2 (4.06 KB, patch)
2013-11-26 11:02 UTC, Jan-Michael Brummer
reviewed Details | Review

Description David Woodhouse 2013-08-02 09:36:57 UTC
Please enable spell-checking on the OOO message editor.
Comment 1 David Woodhouse 2013-08-02 10:49:32 UTC
In fact, perhaps this should be an *HTML* editor and we should setting the messages as HTML?

cf. bug 705337
Comment 2 Milan Crha 2013-08-06 14:10:42 UTC
I'm not in favour for the HTML editor, there is no enough space for it there, but I agree that having a spell-checking there will be nice. There is no spell-checked for GtkTextView currently, or at least I do not know about any, but as I'll need it too, I'll write it one day (we have an ESpellEntry already, based on the libsexy). I'm making this a dependency of bug #329616
Comment 3 Jan-Michael Brummer 2013-11-19 14:54:23 UTC
Created attachment 260249 [details] [review]
Patch adding gtkspell support allowing spell checking for OOO text views

Instead of adopting ESpellEntry to an ESpellTextView within evolution-data-server it is much easier/simpler to depend on gtkspell to allow spell checking of text views. In cases the requirement is not wanted it might be possible to import the library functionality, but this would of cause increase the maintenance work.

Waiting for comments.
Comment 4 Milan Crha 2013-11-20 16:10:48 UTC
Review of attachment 260249 [details] [review]:

Thanks for the patch, it's a very nice start (and pretty simple). I'd like to change/extend some things, though:
a) the one in configure (see below)
b) the preselected language might not be the one based on the current system locale, but the one chosen in Composer (composer supports multiple languages at once, thus pick one used there)
c) the Languages submenu shows en_CA and similar names, instead of user-friendly names, like "English (Canada)"

::: configure.ac
@@ +276,3 @@
+	[AS_HELP_STRING([--enable-gtkspell],
+	[Enable gtkspell @<:@default=no@:>@])],
+	[enable_gtkspell="$enableval"], [enable_gtkspell="no"])

default to 'yes', thus others will notice the new feature
Comment 5 Jan-Michael Brummer 2013-11-26 11:02:19 UTC
Created attachment 262824 [details] [review]
Patch adding gtkspell support allowing spell checking for OOO text views - v2

Based on the review I've changed the configure default value of gtkspell to 'yes'. Furthermore I've retrieved the list of composer mail languages and instruct gtkspell to use the first language in the list. Regarding the "en_US", "de_DE" entries there is nothing we can do about it yet.

If this is a serious problem we need to talk to the maintainer of gtkspell.
Comment 6 Milan Crha 2013-11-26 17:49:04 UTC
Review of attachment 262824 [details] [review]:

Thanks for an update, this makes it half way where I'd like to have it. We can commit this (though see below), and then enhance the popup menu, for which I expected the gtkspell_get_suggestions_menu() would be used, though the worst case would be to copy the popup menu code from gtkspell and only replace the labels with languages with something more user friendly, similar to [1]. I want that to be done in some reusable way, because it'll be used in evolution itself as well (thus it might be useful to add some generic code into evolution/e-util/ and use it in ews).

[1] https://git.gnome.org/browse/evolution/tree/modules/mail/em-composer-prefs.c#n192

::: src/configuration/e-mail-config-ews-ooo-page.c
@@ +590,3 @@
 	widget = gtk_text_view_new ();
+#ifdef WITH_GTKSPELL
+	gchar **languages = spell_get_languages ();

This produces a compiler warning:
e-mail-config-ews-ooo-page.c: In function 'mail_config_ews_ooo_page_constructed':
e-mail-config-ews-ooo-page.c:592:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
  gchar **languages = spell_get_languages ();
  ^
Comment 7 Milan Crha 2013-11-26 17:52:27 UTC
(In reply to comment #6)
> I want that to be done in some reusable way, because it'll be used in
> evolution itself as well (thus it might be useful to add some generic
> code into evolution/e-util/ and use it in ews).

By the way, that would mean that the dependency on gtkspell will be added to evolution, and ews will use only a public e-util API to attach/detach the gtkspell, while not knowing it's gtkspell at all.
Comment 8 Milan Crha 2013-11-28 08:07:48 UTC
Jan-Michael, I would like to commit this on Friday (tomorrow), the initial part (without popup menu customization, I wanted to do that as the next step anyway).

Are you working on it, or I can just pick your work and distribute it into evolution and evolution-ews? My idea is to introduce evolution/e-util/e-spell-text-view.c/.h which will provide only simple interface of two functions:
   
   typedef void ESpellTextView;

   ESpellTextView *e_spell_text_view_attach (GtkTextView *text_view);
   void e_spell_text_view_detach (ESpellTextView *spell);

where the ESpellTextView is just a placeholder for GtkSpell, which I do not want to expose outside of the .c file, for cases where evolution would be compiled without gtkspell.

The e_spell_text_view_attach() will do what you currently have in the evolution-ews code, including the selected language read from GSettings. In the next step (out of this bug report), it'll also make the language popup menu with user-friendly language names.
Comment 9 Milan Crha 2013-11-28 21:12:45 UTC
Ouch, one thing I didn't notice, you use gtkspell for gtk2, but evolution is gtk3 based application, thus the newer gtkspell should be used. It also seems to provide language names for free, thus one gains two things in once.
Comment 10 Milan Crha 2013-11-29 12:15:17 UTC
Created commit f19bffb in ews master (3.11.3+)

All the credits go to Jan-Michael Brummer, because the Evolution interface for GtkTextView spell checker was completely based on his work and idea of using GtkSpell.

Note: This requires commit e607ae8 from evolution git master (see bug #329616)