Failure to insert accented characters on dive site

[p-neves]

Describe the issue:

• [X ] Bug
• [ ] Change request
• [ ] New feature request
• [ ] Discussion request

Issue long description:

It seems it's not possible to insert Locations with accented characters in portuguese. When I try to insert e.g. "á", "à", "é", "ã" or "õ", nothing gets inserted. I can, however, insert "ë"...

Operating system:

Arch Linux

Subsurface version:

4.7.8 compiled from source

Yes

Compiled myself

4.7.8-356-gc64f421006a4

Steps to reproduce:

1) Create a dive (or import dive from dive computer) 2) Edit location by inserting a new location with some accented characters

Current behavior:

The accented characters are not displayed.

Expected behavior:

It should be possible to insert all characters on names for the locations (inserting these characters on the notes, buddy or divemaster fields works fine...

Additional information:

Mentions:

[mfbernardes]

I can reproduce on Ubuntu 18.04. Works fine on Mac.

The behavior I observed on linux was:

• The first character you type after you select Location text entry can be accented.
• After the tooltip is open no accented characters is accepted anymore.

I'm guessing it's related to custom line edit at locationinformation.cpp, but didn't investigate.

[dirkhh]

The label name is wrong - should be "release blocker"

[bstoeger]

Can't reproduce on Kubuntu 18.4 (compiled from source).

[dirkhh]

I can't reproduce it on Arch which makes it a lot harder to fix :-(

[neolit123]

i will play with this. seems like a locale issue.

[neolit123]

@mfbernardes @p-neves

i can't reproduce this on both Linux and Windows with Qt 5.9.x. tried changing locales and playing with keyboards to no avail.

i'm going to have to ask you to help to debug this...

first, some questions: 1) what Qt version are you building against? 2) Does the same happen if your run our AppImage: https://github.com/Subsurface-divelog/subsurface/releases/download/continuous/Subsurface-4.7.8-414-g86988f9cdcca-x86_64.AppImage 3) are you both running Portuguese locale, or @mfbernardes is your in Spanish? 4) can you CTRL+V special characters in there instead of typing them? 5) if you click the Globe next to the location field can you edit the location with special characters in there? i'm going to assume yes?

also after answering the above, please try this patch over the current Git master and tell me what happens after building / running: _test-dive-location.txt

git apply _test-dive-location.txt

[mfbernardes]

@neolit123

From I what could see, this only happens when the tooltip is open.

Answers:

1. 5.9.5, from Ubuntu 18.04. Tried also 5.10.1 from qt.io, same result.
2. Yes, same thing.
3. I use en_US everywhere, us_intl keyboard. I tested with pt_BR as well with the same result.
4. Yes, Ctrl+V works.
5. Yes, no problem there.

With your patch the tooltip never shows up, so it works fine.

[neolit123]

@mfbernardes thank you for the feedback. now we need to figure out how the tooltip is blocking the special characters.

[dirkhh]

So how is the special character input done on Linux (there are a few different ways to do this, depending on which physical keyboard you have). For example, if you have a German keyboard, I bet you can easily enter ä and ß even with the tooltip. But my guess is that with a US keyboard and a compose key (so you have to enter compose key, ", a - three keystrokes) this fails. And I'm guessing that somehow our tooltip is breaking the connection of these three keystrokes into one.

I repeat, that's wild guesses, I'm having some problems with my Linux VM right now so haven't actually tried this.

[bstoeger]

That's a very good point - I have never used anything but "nodeadkeys" layouts and I totally forgot that people with phonetic alphabets (i.e. outside of China / Japan / Korea) compose their characters with multiple key-strokes.

[neolit123]

@mfbernardes

my patch changed a couple of things: [1]

-   view->installEventFilter(this);
+   // view->installEventFilter(this);

[2]

-   connect(this, &QLineEdit::textEdited, this, &DiveLocationLineEdit::setTemporaryDiveSiteName);
+   // connect(this, &QLineEdit::textEdited, this, &DiveLocationLineEdit::setTemporaryDiveSiteName);

can you please verify if only one of them causes the problem? my guess is that [1] is the problem here.

[neolit123]

also @mfbernardes @p-neves , what are you using to enter these special characters on your keyboard? please describe the process in detail - are you using a modifier key or is it via multiple keystrokes?

i must admit i've never used any of that.

a Bulgarian Phonetic layout translates pretty well to QWERTY and we don't have accents. well we do, but we only use them if we really want to describe where an accent is. the accents are implied in the language. ;)

[dirkhh]

OK, I have my Linux VM sorted (for now, at least), so I was able to reproduce the problem. As expected, using the compose key to create accents fails. I'll try your patch next and report back.

[mfbernardes]

@neolit123 I use two keystrokes. 'a-> á, 'e -> é, ~n -> ñ, "a->ä

I've used ñ (dives in Philippines, this is not used in Portuguese), but this works find on my Mac.

Keyboard is configured as US international (w/ dead keys).

@dirkhh @bstoeger @neolit123 When using multiple keystrokes a QInputMethodEvent is sent to the widget.

Looks like the default is ignore the input, or inputMethodEvent() needs to be implemented. I just could not understand what inputMethodEvent() is supposed to do.

[dirkhh]

Applying the patch fixes it. Using just the first hunk (so leaving the connect call enabled) hangs my UI (I cannot interact with Subsurface or anything else after the first character typed into that text box - have to switch to TTY and kill Subsurface). Using just the second hung (commenting out the connect call) is enough to get accents / umlauts in the location field -- but in return we no longer offer completions.

[neolit123]

Ubuntu's official way of installing custom keyboards is broken for me. Returns silly errors. on screen keyboards obviously work, so it's an input method issue (QInputMethodEvent) like @mfbernardes points out.

investigating possible solutions. this bug probably lurked for quite some time, yet i don't understand how it only happens on Linux.

[mfbernardes]

For some reason QInputMethodEvent don't show up on Mac.

My understanding is: with tooltip hidden this event is handled by QLineEdit. With tooltip visible DiveLocationListView receives the event and doesn't know what to do with it.

Time for some sleep.

[dirkhh]

So my guess is that if DiveLocationListView receives this event, it should just pass it on to the QLineEdit...

[mfbernardes]

@dirkhh Can you try this patch? inputmethodevent-handle.txt

This basically sends the event to be processed by QLineEdit.

Worked for me, but some extra checks might be needed (as to ensure obj is DiveLocationListView).

[dirkhh]

Yep, that fixed it. Awesome! Can you create a PR for this, please?

[dirkhh]

(I won't mention that the patch doesn't follow coding style...) :-)

[neolit123]

thanks @mfbernardes ,

+   else if (e->type() == QEvent::InputMethod) {
+       QInputMethodEvent * keyEvent = static_cast< QInputMethodEvent * >( e );
+       this->inputMethodEvent( keyEvent);
+   }

so i think when the list view becomes visible it starts receiving the InputMethod events, but our event filter does not handle them. this addition should work.

i guess it remains a mystery why it doesn't work on Linux only.

[mfbernardes]

@dirkhh PR created.

hope it's ok(-ish) now in terms of codingStyle.

[dirkhh]

I'm on a plane from Beijing to Seattle. Will look when I'm back home in about 14 hours... Or one of the other maintainers will provide feedback / merge :-)

[bstoeger]

i guess it remains a mystery why it doesn't work on Linux only.

This is strange indeed. One would think that the catch-all return false would act as if no filtering happened...?

[bstoeger]

Oh - I misunderstood. The thing is, this catches events from the DiveLocationListView and passes it to the DiveLocationLineEdit.

The magic line in the QEvent::KeyPress path is

    event(e);

This moves the event to DiveLocationLineEdit.

So for consistency, I guess one could do the same:

    else if (e->type() == QEvent::InputMethod) {
        event(e);
    }

Well - too late now. :)

[steffen-kdab]

This is still not fixed. I can't type accented characters into the location (and other) fields with compose/dead keys. Tested on Ubuntu 22.04.1 LTS with Subsurface 5.0.9 AppImage latest, but it was a problem in earlier versions too, so I don't think it's a regression.

Dead keys work fine in the Notes QTextEdit though, only the QLineEdits seem to be broken.

[dirkhh]

Your problem is likely that you are using the AppImage. Ubuntu has native binaries which tend to create a much better user experience. I just tested this and here locally I can use compose in the location field just fine with a Ubuntu system.

[p-neves]

On 09/08/22 01:14, Dirk Hohndel wrote:

Your problem is likely that you are using the AppImage. Ubuntu has native binaries which tend to create a much better user experience. I just tested this and here locally I can use compose in the location field just fine with a Ubuntu system.

— Reply to this email directly, view it on GitHub https://github.com/subsurface/subsurface/issues/1405#issuecomment-1208748880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2F43UDBLHRWDYHTPIW73DVYGPGFANCNFSM4FFQ63JA. You are receiving this because you were mentioned.Message ID: @.***>

Hi Dirk:

I'm using the latest master on Arch Linux and I cannot type accented characters on the Location nor on the Buddy fields. I can, however, type such characters on the Dive Guide field....

Cheers:

Pedro

[bstoeger]

I'm using the latest master on Arch Linux and I cannot type accented characters on the Location nor on the Buddy fields. I can, however, type such characters on the Dive Guide field....

This is strange, because the buddy and dive guide fields use the same class TagWidget.

I never used keyboard layouts with composing keys. However, I changed to such a layout on my laptop and I could enter accents in both fields. That's on Kubuntu, freshly compiled from mmaster. Very weird.

[steffen-kdab]

Your problem is likely that you are using the AppImage. Ubuntu has native binaries which tend to create a much better user experience. I just tested this and here locally I can use compose in the location field just fine with a Ubuntu system.

Hi Dirk.

I tried the Ubuntu PPA instead, and the problem is similar, but stranger:

Compose doesn't work most of the time in the Location, Buddy, Diveguide,... fields, ie. Compose+o+/ just becomes 'o/' and not 'ø'. Now if I clear the field, defocus it and focus it back, I can sometimes get it to work correctly. In the Location field only for the first character, but in the other fields for any character :-O

[bstoeger]

I now can reproduce this. Reopen (though the actual cause might be different from the original report).

[bstoeger]

The problem is the interaction of the completers with Qt's composition handling. In fact, this patch:

diff --git a/desktop-widgets/tagwidget.cpp b/desktop-widgets/tagwidget.cpp
index ad5ff7c38..9f9b49725 100644
--- a/desktop-widgets/tagwidget.cpp
+++ b/desktop-widgets/tagwidget.cpp
@@ -37,10 +37,10 @@ TagWidget::TagWidget(QWidget *parent) : GroupedLineEdit(parent), m_completer(NUL

 void TagWidget::setCompleter(QCompleter *completer)
 {
-       m_completer = completer;
-       m_completer->setWidget(this);
-       connect(m_completer, SIGNAL(activated(QString)), this, SLOT(completionSelected(QString)));
-       connect(m_completer, SIGNAL(highlighted(QString)), this, SLOT(completionHighlighted(QString)));
+       //m_completer = completer;
+       //m_completer->setWidget(this);
+       //connect(m_completer, SIGNAL(activated(QString)), this, SLOT(completionSelected(QString)));
+       //connect(m_completer, SIGNAL(highlighted(QString)), this, SLOT(completionHighlighted(QString)));
 }

 #if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)

fixes the problem for the TagWidgets (but obviously makes the completers disfunctional).

I suspect that the completers messing with the focus makes composition break.

[bstoeger]

Some more analysis for the TagWidget:

When pressing a dead-key, we get a textChanged signal, which calls reparse(). There, the statement m_completer->complete(); replaces the dead key pseudo-character by a real character.

Best thing is probably to look at the Qt classes such as QLineEdit, how they handle the problem.

PS: The code is completely broken anyway, a single key stroke can lead to a cascade of six reparse() calls.

[bstoeger]

Having looked at the QLineEdit source, it appears that one shouldn't hook into the textChanged signal, but rather the inputMethodEvent() virtual function.

[bstoeger]

Please check if #3486 improves the situation for TagWidgets (not the location).

[bstoeger]

Jeez. When the popup is open, the input field doesn't get inputMethodEvent() calls, but keyPressEvent() calls, which means that this doesn't work as long as the popup is open.

I fear I will not be able to debug this in the next two weeks. I have the impression that the best thing is probably to rewrite the TagWidget from scratch.

[bstoeger]

Improved version of #3486 now sets the WA_InputMethodEnabled flag on the popup explicitly. Seems to work for me. Please check.

[p-neves]

On 13/08/22 18:29, bstoeger wrote:

Improved version of #3486 https://github.com/subsurface/subsurface/pull/3486 now sets the |WA_InputMethodEnabled| flag on the popup explicitly. Seems to work for me. Please check.

— Reply to this email directly, view it on GitHub https://github.com/subsurface/subsurface/issues/1405#issuecomment-1214194052, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2F43TYOA6YWPMGORCPMPTVY7LQZANCNFSM4FFQ63JA. You are receiving this because you were mentioned.Message ID: @.***>

Hi:

This works for me on the Buddy field. It doesn't work on the Location field...

Cheers:

Pedro

[bstoeger]

Thanks for testing.

The version for the location field is in #3487. It works for me in cursory testing. However, this all feels very "wrong".

[bstoeger]

This works for me on the Buddy field. It doesn't work on the Location field...

Could you please test current master?

[p-neves]

On 28/08/22 18:11, bstoeger wrote:

This works for me on the Buddy field. It doesn't work on the
Location field...

Could you please test current master?

Hi!

Works for me, on both fields.

Thanks a lot

Pedro

[bstoeger]

Thanks, closing for now (until next time!).