Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Cannot activate sa-iast input method #52

Closed
alaymari opened this issue Sep 16, 2022 · 15 comments
Closed

[BUG] Cannot activate sa-iast input method #52

alaymari opened this issue Sep 16, 2022 · 15 comments
Assignees
Labels

Comments

@alaymari
Copy link

alaymari commented Sep 16, 2022

**I frequently use kn-kgp.mim, sa-itrans.mim and sa-iast.mim to input text. Everything was working fine. The latest update resulted in me not being able to use sa-iast.mim for inputting text. The other two work as expected. I noticed this behaviour even on freshly created user account.

Found out that on one machine with an older version, everything works correctly.**
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'Choose sa-iast" as the input method.'
  2. That keyboard type will not be activated, it reverts to EN.

Expected behavior
The chosen keyboard method is to become active.

ibus-m17n version?
1.4.11-1 (not working) ; 1.4.9-1+b1 (working)

m17n-db versions?
1.8.0-3

ibus version?
1.5.27

Distribution and version?
Debian testing

Desktop and version?
i3, cinnamon

Xorg or Wayland?
Xorg

@mike-fabian
Copy link

I can reproduce this.

@mike-fabian
Copy link

The reason why this works for all other engines but not for this one is actually because this is the only m17n engine which has an uppercase name:

$ ibus list-engine | grep m17n:sa:
  m17n:sa:harvard-kyoto - sa-harvard-kyoto (m17n)
  m17n:sa:IAST - sa-IAST (m17n)
  m17n:sa:inscript2 - sa-inscript2 (m17n)
  m17n:sa:itrans - sa-itrans (m17n)

@mike-fabian
Copy link

The parsing in
https://github.com/ibus/ibus-m17n/blob/main/src/engine.c#L130
is screwed up if the name of the input method is in upper case:

static gboolean
ibus_m17n_scan_class_name (const gchar *class_name,
                           gchar      **lang,
                           gchar      **name)
{
    gchar *p;

    g_return_val_if_fail (g_str_has_prefix (class_name, "IBusM17N"), FALSE);
    g_return_val_if_fail (g_str_has_suffix (class_name, "Engine"), FALSE);

    /* Strip prefix and suffix */
    p = *lang = g_strdup (class_name + 8);
    p = g_strrstr (p, "Engine");
    *p = '\0';

    /* Find the start position of <Name> */
    while (!g_ascii_isupper (*--p) && p > *lang)
        ;
    g_return_val_if_fail (p > *lang, FALSE);
    *name = g_strdup (p);
    *p = '\0';

    *lang[0] = g_ascii_tolower (*lang[0]);
    *name[0] = g_ascii_tolower (*name[0]);

    return TRUE;
}

@mike-fabian
Copy link

For sa-itrans, the class name is IBusM17NSaItransEngine.
For sa-IAST, the class name is IBusM17NSaIASTEngine.
And then while (!g_ascii_isupper (*--p) && p > *lang); does not work.

@mike-fabian
Copy link

And then this code in ibus_m17n_engine_class_init () fails:

https://github.com/ibus/ibus-m17n/blob/main/src/engine.c#L235

    if (!ibus_m17n_scan_class_name (G_OBJECT_CLASS_NAME (klass),
                                    &lang, &name)) {
        g_free (lang);
        g_free (name);
        return;
    }
    klass->gsettings = g_settings_new_with_path (
        "org.freedesktop.ibus.engine.m17n",
        g_strdup_printf ("/org/freedesktop/ibus/engine/m17n/%s/%s/",
                         lang, name));
    MPlist *l = minput_get_title_icon (msymbol (lang), msymbol (name));
    if (l && mplist_key (l) == Mtext) {
        klass->title = ibus_m17n_mtext_to_utf8 (mplist_value (l));
    }
    else {
        klass->title = NULL;
    }
    MPlist *n = mplist_next (l);
    if (n && mplist_key (n) == Mtext) {
        klass->icon = ibus_m17n_mtext_to_utf8 (mplist_value (n));
    }
    else {
        klass->icon = NULL;
    }

@mike-fabian
Copy link

MPlist *l = minput_get_title_icon (msymbol (lang), msymbol (name));

This becomes a NULL pointer because the parsing of lang and name from the class name failed so badly.

And then MPlist *n = mplist_next (l); segfaults.

@mike-fabian
Copy link

I have test builds for Fedora 35/36/37/rawhide here:

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/builds/

which fix the problem.

I do a bit more testing and probably make a new release soon.

@mike-fabian
Copy link

This video shows that it works again for me now:

Peek.2022-09-16.21-53.mp4

@mike-fabian
Copy link

In the video on can see that sa-IAST gets a generic ⚙️ icon in the ibus property panel and the tooltip is wrong, it shows m17n:saIAS:t because of that parsing error in the class name. It should be m17n:sa:IAST.

So because of that parse error the icon and the tooltip are wrong. But ibus-m17n-1.4.9 didn’t even try to show different icons in the ibus-property panel and no tooltips either. All property panels for all engines had the same ⚙️ icon and no tooltip.

Trying to make this nicer caused the problem for sa-IAST because the parsing used to get the nicer icon and the tooltip failed and caused the segfault.

My current fix gets the nicer icon and correct tooltips for all engines except sa-IAST now, for sa-IAST the crash is fixed and it is as good as before.

@alaymari
Copy link
Author

alaymari commented Sep 17, 2022 via email

@alaymari
Copy link
Author

In the file /usr/share/m17n/sa-iast.mim, I changed (input-method sa IAST ) to (input-method sa iast ), and renamed the file /usr/share/m17n/icons/sa-IAST.png to /usr/share/m17n/icons/sa-iast.png. Restarted ibus, and it works on my debian system :-)

@mike-fabian
Copy link

mike-fabian commented Sep 17, 2022

In the file /usr/share/m17n/sa-iast.mim, I changed (input-method sa IAST ) to (input-method sa iast ), and renamed the file /usr/share/m17n/icons/sa-IAST.png to /usr/share/m17n/icons/sa-iast.png. Restarted ibus, and it works on my debian system :-)

Yes, renaming works of course, and I think the renaming you did is actually also the best solution.

But I will nevertheless include a fix in the next release of ibus-m17n to make it work even with the original naming of sa-IAST.

Changing the naming upstream in m17n is difficult and I have a very ugly hack now which makes it work without renaming.

Not only does it avoid the crash when switching to sa-IAST, it also shows the correct icon and makes the settings for sa-IAST work (The settings were broken as well).

This screenshot shows that both the settings and the icon work (without renaming) with my latest test builds 1.4.16 from:

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/builds/

Screenshot

@mike-fabian
Copy link

@mike-fabian
Copy link

**I frequently use kn-kgp.mim, sa-itrans.mim and sa-iast.mim to input text.

By the way, did you know that you can use all these *.mim input methods also with ibus-typing-booster?

https://github.com/mike-fabian/ibus-typing-booster
https://mike-fabian.github.io/ibus-typing-booster/

ibus-typing-booster can simulate the behaviour of ibus-m17n by switching off all the extra features:

https://mike-fabian.github.io/ibus-typing-booster/docs/user/#2_2_1_1

Even when one does not need the extra features of ibus-typing-booster, it maybe be better to use it instead of ibus-m17n because it is more actively maintained and has less bugs.

By default it also has a lot of extra features like word completion which can be very useful.

Because of this sa-IAST issue here, I checked whether sa-IAST works with ibus-typing-booster. It works, but there is a small bug
in the setup tool, when adding sa-IAST the search does not work well one cannot search for "iast" or "IAST". Searching for "sa-" works and scrolling without searching works. See:

mike-fabian/ibus-typing-booster#387

@alaymari
Copy link
Author

alaymari commented Sep 18, 2022 via email

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this issue Sep 18, 2022
https://build.opensuse.org/request/show/1004408
by user Pi-Cla + dimstar_suse
update: 1.4.13 -> 1.4.17
- Fix problem that sa-IAST input method cannot be activated and make settings of sa-IAST work
  * ibus/ibus-m17n#52
- Let IBusM17nEngine inherit from IBusEngineSimple to enable compose support
  * ibus/ibus-m17n#51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants