Mimic QInputControl::isAcceptableInput() when filtering typed characters
ClosedPublic

Authored by ahmadsamir on Aug 26 2019, 6:49 PM.

Details

Summary

Move all input characters filtering out of typeChars() to KateViewInternal
and filter the input before sending it to typeChars().

This increases the scope of unicode characters that users can type in ktexteditor.
For more info see:
QChar documentation
http://www.unicode.org/Public/UNIDATA/UnicodeData.txt

This should fix:

  • bug 396764 (typing soft-hyphens)
  • bug 366424 (typing "private use" unicode characters)
  • Hopefully bug 389796 (typing formatting characters such as ZWNJ)

BUG: 396764
BUG: 366424
BUG: 389796

Test Plan

Test typing a soft-hyphen char (here I used Compose key + minus + minus + space)
For the two other bugs, ask the users to test...

All unit tests passed

Diff Detail

Repository
R39 KTextEditor
Branch
ahmad/soft-hyphen (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15682
Build 15700: arc lint + arc unit
ahmadsamir created this revision.Aug 26 2019, 6:49 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptAug 26 2019, 6:49 PM
ahmadsamir requested review of this revision.Aug 26 2019, 6:49 PM

This is a good idea.
But actually, is there any reason we filter out stuff at all?

I checked a bit what Qt does:

bool QInputControl::isAcceptableInput(const QKeyEvent *event) const
{

const QString text = event->text();
if (text.isEmpty())
    return false;

const QChar c = text.at(0);

// Formatting characters such as ZWNJ, ZWJ, RLM, etc. This needs to go before the
// next test, since CTRL+SHIFT is sometimes used to input it on Windows.
if (c.category() == QChar::Other_Format)
    return true;

// QTBUG-35734: ignore Ctrl/Ctrl+Shift; accept only AltGr (Alt+Ctrl) on German keyboards
if (event->modifiers() == Qt::ControlModifier
        || event->modifiers() == (Qt::ShiftModifier | Qt::ControlModifier)) {
    return false;
}

if (c.isPrint())
    return true;

if (c.category() == QChar::Other_PrivateUse)
    return true;

if (m_type == TextEdit && c == QLatin1Char('\t'))
    return true;

return false;

}

is applied on key events.

cullmann requested changes to this revision.Aug 26 2019, 7:34 PM

I think the proper fix is:

  1. remove any filtering in typeChars().
  2. implement a Qt like handling in the void KateViewInternal::keyPressEvent(QKeyEvent *e) member.

Could you try that?

This revision now requires changes to proceed.Aug 26 2019, 7:34 PM

Digging around in git log I found: https://git.reviewboard.kde.org/r/127026/

But I think that's covered by the method in QInputControl, and ktexteditor has a unit test for that; I'll see how it goes.

ahmadsamir retitled this revision from Enable typing soft-hyphen characters to Mimic QInputControl::isAcceptableInput() when filtering typed characters.
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir edited the test plan for this revision. (Show Details)

Rework the whole patch

Improve comment in header file

Digging around in git log I found: https://git.reviewboard.kde.org/r/127026/

But I think that's covered by the method in QInputControl, and ktexteditor has a unit test for that; I'll see how it goes.

Nice catch. In the review request you refer to there is also a unit test listed. Would it be possible to extend this one?

Please only add the isAcceptableInputText with the event.
We only need to check that for the keyboard input in KateViewInternal::keyPressEvent(QKeyEvent *e).
And there we don't need to do this for the \t tab insertion, makes no sense, we want to add there a \t always.
Perhaps the \t/\r\n handling can even be removed, not sure we handle that this way in key event.

cullmann accepted this revision.Aug 27 2019, 5:03 PM

Actually, I can take care of the changes after applying this, is already a good start, thanks!

This revision is now accepted and ready to land.Aug 27 2019, 5:03 PM
This revision was automatically updated to reflect the committed changes.