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 604159 - Spell checker breaks words incorrectly (on apostrophes, for example)
Spell checker breaks words incorrectly (on apostrophes, for example)
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat
2.29.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
: 607032 615089 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-12-09 11:04 UTC by Will Thompson
Modified: 2010-05-25 09:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to fix and improve spell checking. (11.38 KB, patch)
2010-05-13 08:34 UTC, Mike Ruprecht
reviewed Details | Review
Patch to fix and improve spell checking. (11.70 KB, patch)
2010-05-19 04:40 UTC, Mike Ruprecht
reviewed Details | Review
Patch to fix and improve spell checking. (11.71 KB, patch)
2010-05-23 08:12 UTC, Mike Ruprecht
none Details | Review

Description Will Thompson 2009-12-09 11:04:47 UTC
If I type “isn't” into my IM window, Empathy marks the “isn” as misspelled. This seems to be because the spell checking code is really stupid. The algorithm goes something like this:

  on GtkTextBuffer:changed (yes, every time):
    guess where words start and end using gtk_text_iter_forward_word_end();
    pass each word to enchant

Now, the documentation for gtk_text_iter_forward_word_end() does say “Word breaks are determined by Pango and should be correct for nearly any language (if not, the correct fix would be to the Pango word break algorithms).” That said, I find it hard to believe that this is the best way to spell-check text...

Relatedly, if you do this:

• type a few words into the entry;
• go back to the start, and type some more words there

then any new spaces you insert are marked as misspellings.
Comment 1 Jeff Wheeler 2010-02-20 18:57:50 UTC
If I understand it correctly, a lot of apps use the GtkSpell widget to handle this automatically. Is there any reason Empathy can't use this widget, instead of handling spell-checking itself?
Comment 2 Diego Escalante Urrelo (not reading bugmail) 2010-03-03 07:17:04 UTC
*** Bug 607032 has been marked as a duplicate of this bug. ***
Comment 3 Guillaume Desmottes 2010-04-08 10:34:11 UTC
*** Bug 615089 has been marked as a duplicate of this bug. ***
Comment 4 Mike Ruprecht 2010-04-10 09:02:17 UTC
My guess is the reason GtkSpell isn't used is because Empathy supports using multiple dictionaries at once, whereas GtkSpell 2 does not. However, GtkSpell 3 looks like it would be perfect for Empathy's use-case... Only problem is GtkSpell 3 looks like it's dead in the water... :/

I did try stuffing GtkSpell 2 in. It does allow you to switch between languages with the context menu, but that's probably an unacceptable regression from our current functionality.

I did some searching and stumbled across GtkSpell 2's source in that it actually has a special case on top of gtk_text_iter_forward_word_end where it checks if there's an apostrophe, and if there is it keeps going.

Long story short, I copied those two functions and it seems to work pretty well now. :) See branch here: http://git.collabora.co.uk/?p=user/maiku/empathy.git;a=shortlog;h=refs/heads/spell-check
Comment 5 Guillaume Desmottes 2010-04-12 08:16:55 UTC
Looks good to me. But what's the license of GtkSpell 2? empathy-chat is currently GPL but we'd like to move to LGPL at some point.
Comment 6 Mike Ruprecht 2010-04-12 18:38:10 UTC
GtkSpell2 appears to be GPL.

Also, I was testing out this patch more and there still seems to be some issues with it. I think I need to work on this some more. If I have to drop those functions, that's fine too I can start clean.
Comment 7 Mike Ruprecht 2010-05-13 08:34:09 UTC
Created attachment 160955 [details] [review]
Patch to fix and improve spell checking.

I rewrote my branch above and here's the patch for convenience.
Comment 8 Guillaume Desmottes 2010-05-17 14:18:48 UTC
Review of attachment 160955 [details] [review]:

I see you now have 2 branches: spell-check and spell-check-2. Which one is the right one?

::: libempathy-gtk/empathy-chat.c
@@ +96,3 @@
 
+	gboolean	   spell_checking_enabled;
+	guint		   insert_text_id;

Please add comments explaining the semantic of those variables.
Comment 9 Mike Ruprecht 2010-05-19 04:40:43 UTC
Created attachment 161403 [details] [review]
Patch to fix and improve spell checking.

Ok, here's a patch updated per your comments. Also, the spell-check branch is the correct one. Sorry for the ambiguity. :)
Comment 10 Guillaume Desmottes 2010-05-19 07:47:57 UTC
Review of attachment 161403 [details] [review]:

Looks good; just a small comment. You should also include the bug number in your commit message.

::: libempathy-gtk/empathy-chat.c
@@ +101,3 @@
+
+	/* These store the signal handler ids for the enclosed text entry. */
+	guint		   insert_text_id;

sig handlers are gulong, not guint.
Comment 11 Mike Ruprecht 2010-05-23 08:12:26 UTC
Created attachment 161758 [details] [review]
Patch to fix and improve spell checking.

Updated the patch per your review comments. :)
Comment 12 Guillaume Desmottes 2010-05-25 09:40:23 UTC
Thanks; merged to master!

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.