moebiuscurve / ibus-table-others

ibus-table-others
code.google.com/p/ibus/
GNU General Public License v3.0
15 stars 8 forks source link

crash on `_0` for ̥ in ipa-x-sampa table #21

Closed silmeth closed 3 years ago

silmeth commented 3 years ago

Whenever I try to write something like m̥ using the ipa-x-sampa table by inputting m_0, the table engine stops. I can still input this by navigating the hint list using arrows and choosing the correct option.

Here’s my ~/.cache/ibus-table/debug.log after setting debug level /usr/libexec/ibus-engine-table to 3: https://gist.github.com/silmeth/a640fa4c71669f26cdc4459c9180207b

silmeth commented 3 years ago

I use Kubuntu 20.10 on x64, I have ibus, ibus-table, and ibus-table-ipa-x-sampa from Ubuntu repos, versions of packages:

ibus: 1.5.23-0ubuntu1
ibus-table: 1.11.0-1
ibus-table-ipa-x-sampa: 1.3.11-2
silmeth commented 3 years ago

Here log when successfully inputting this text using arrows to choose the correct candidate from the list: https://gist.github.com/silmeth/c69f53e8cd46e8391cf2283c79c1c499

mike-fabian commented 3 years ago

It works for me on Fedora 34 with the following package versions:

[mfabian@fedora ~]$ cat /etc/fedora-release 
Fedora release 34 (Thirty Four)
[mfabian@fedora ~]$ rpm -q ibus
ibus-1.5.24-1.fc34.x86_64
[mfabian@fedora ~]$ rpm -q ibus-table
ibus-table-1.12.4-5.fc34.noarch
[mfabian@fedora ~]$ rpm -q ibus-table-latin
ibus-table-latin-1.3.11-4.fc34.noarch
[mfabian@fedora ~]$ rpm -qf /usr/share/ibus-table/tables/ipa-x-sampa.db 
ibus-table-latin-1.3.11-4.fc34.noarch
[mfabian@fedora ~]$ 

Screenshot

mike-fabian commented 3 years ago

Apparently you have an older version of ibus-table, you have ibus-table: 1.11.0-1 and I have ibus-table-1.12.4-5.fc34.noarch.

Do you also see the F1, F2, ... in front of the candidates or do you see 1, 2, ... ?

silmeth commented 3 years ago

Do you also see the F1, F2, ... in front of the candidates or do you see 1, 2, ... ?

F1., F2., etc. Also, other combinations with digits work without problems, eg. h_1 for h₁.

ibus_candidate_list

Above using F7 to input m̥ worked too. It’s just writing _0 that stops ibus-table.

Will try newer ibus-table later.

mike-fabian commented 3 years ago

I am just guessing, I don't want to install an older version of ibus-table to check, but my suspicion is that in an older version of ibus-table, not only F1, F2,F3,... selected candidates of ipa-x-sampa but also 1,2,3 ... And 0 selected the candidate with number 0, which didn't exist and that might have cause the problem.

You attached the log when it worked sucessfully using the arrow keys.

More interesting would be the log when it doesn't work, there might be an error message in the log when you type the 0.

But I guess updating ibus-table will fix it, if updating fixes it, I would not search what was wrong in the old version.

mike-fabian commented 3 years ago

I think part of this bug is still there in the current ibus-table-1.12.4.

It just accidentally works for ipa-x-sampa because in the new version it does not react to 0, 1, 2, 3... anymore.

But if you use a candidate list of length 6 and type F7, it still crashes.

mike-fabian commented 3 years ago

If you manage to select a candidate outside of the visible range using a key binding, it crashes.

silmeth commented 3 years ago

More interesting would be the log when it doesn't work, there might be an error message in the log when you type the 0.

The gist linked in the first comment is a log of unsuccessful try when it crashed when I typed m_0.

> But if you use a candidate list of length 6 and type F7, it still crashes.

Hmm, it doesn’t seem to be the case for me. I tried using F7 on a short candidate list and it was just ignored (eg. dF7 does nothing, the F7 keystroke is just ignored), ibus-table kept working and allowed me to write more.

Also tried with another table (my own for Cyrillic) which uses numbers for selecting candidates, and here also pressing eg. 9 on a candidate list with 7 candidates did not crash it (pressing s9 comitted the с9 string and ibus table kept working).

Nevermind, I misunderstood what you meant.

silmeth commented 3 years ago

Oh, I think I misunderstood you.

You mean in the case when the candidate list is actually longer but the presented list has eg. only candidates in the range 1–7, and then you chose something outside that range (like 0 or 9) – it crashes?

silmeth commented 3 years ago

Yes – when I limit the candidate list to eg. max. 5 candidates and try using F7 it crashes the same way. The same happens when using my Cyrillic table if I hit 0 when presented any candidate list. So indeed it seems to be this issue.

I guess it should rather be filed as direct ibus-table issue then. Is there one already?

mike-fabian commented 3 years ago

You mean in the case when the candidate list is actually longer but the presented list has eg. only candidates in the range 1–7, and then you chose something outside that range (like 0 or 9) – it crashes?

Yes ☺

ibus-table-1.12.4 happens to work for typing m_0 with the ipa-x-sampa table, but the actual problem of sometimes being able to choose a candidate outside the range is still there.

No there is no issue yet. I am already working on a fix but it would be nice if you file an ibus-table issue.

mike-fabian commented 3 years ago

This should fix it:

$ git diff 
diff --git a/engine/table.py b/engine/table.py
index 9a01401..9f2f208 100644
--- a/engine/table.py
+++ b/engine/table.py
@@ -3337,9 +3337,11 @@ class TabEngine(IBus.EngineSimple):
         :param number: The number of the candidate
         :type number: Integer
         '''
-        if not self._candidates:
+        if not self._candidates or number > len(self._candidates):
             return False
         index = number - 1
+        if not 0 <= index < self._page_size:
+            return False
         if self.commit_to_preedit_current_page(index):
             self.commit_string(
                 self.get_preedit_string_complete(),
mike-fabian commented 3 years ago

Seems to work well for me.

Forget about opening a new issue for ibus-table, I just refer to the issue here in the release notes for ibus-table 1.12.5.

silmeth commented 3 years ago

Huge thanks for identifying and fixing it so quickly! :)

mike-fabian commented 3 years ago

Maybe you are interested in this:

https://github.com/mike-fabian/m17n-db-ipa-x-sampa

I made this long ago but just uploaded it to github.

It does basically the same as the IPA-X-SAMPA table for ibus-table but you use it with ibus-m17n or ibus-typing-booster instead.