Bug 144412 - Qt5/KF5 flips writing direction on Ctrl+Shift+p
Summary: Qt5/KF5 flips writing direction on Ctrl+Shift+p
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
7.2.0.0 alpha1+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.3.0 target:7.2.2
Keywords:
Depends on:
Blocks: KDE
  Show dependency treegraph
 
Reported: 2021-09-09 15:14 UTC by Jan-Marek Glogowski
Modified: 2021-09-13 05:15 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan-Marek Glogowski 2021-09-09 15:14:08 UTC
This is an other regression from bug 143298.

1. Open Writer
2. Press Ctrl+Shift+P (superscript)

Result: Writer input changes to superscript and the paragraph to RTL?!

Expected result: no change of writing direction

-----

SAL_DEBUG("SalEvent::KeyModChange " << aModEvt.mnCode << " " << (int) aModEvt.mnModKeyCode);
SAL_DEBUG("SalEvent::KeyInput " << aEvent.mnCode << " " << aEvent.mnCharCode);

Adding this debug output reveals for gtk3:

debug:397428:397428: SalEvent::KeyModChange 8192 8
debug:397428:397428: SalEvent::KeyModChange 12288 10
debug:397428:397428: SalEvent::KeyInput 12815 80
debug:397428:397428: SalEvent::KeyUp 12815 80
debug:397428:397428: SalEvent::KeyModChange 8192 0
debug:397428:397428: SalEvent::KeyModChange 0 0

80 = 'P'

For gen:

debug:402712:402712: SalEvent::KeyModChange 8192 0
debug:402712:402712: SalEvent::KeyModChange 12288 0
debug:402712:402712: SalEvent::KeyInput 12815 80
debug:402712:402712: SalEvent::KeyUp 12815 80
debug:402712:402712: SalEvent::KeyModChange 8192 0
debug:402712:402712: SalEvent::KeyModChange 0 0

For kf5:

debug:383771:383771: SalEvent::KeyModChange 8192 8
debug:383771:383771: SalEvent::KeyModChange 12288 10
debug:383771:383771: SalEvent::KeyInput 12815 16
debug:383771:383771: SalEvent::KeyUp 12815 16
debug:383771:383771: SalEvent::KeyModChange 8192 10
debug:383771:383771: SalEvent::KeyModChange 0 8

I have no idea, why pEvent->text().at(0).unicode() returns 16. text().length == 1, so nothing else missing. But that seems to not cause any problem.

Obviously the reported mnModKeyCode on release is wrong (should be 8 and 0) and fixing that fixes the bug.
Comment 1 Jan-Marek Glogowski 2021-09-09 15:41:39 UTC
FWIW with gtk3 I get:

1. Press and release Ctrl+Shift+P => changes superscript
2. Hold Ctrl+Shift
3. Release Ctrl

I always use the modifiers on the right keyboard side.

=> RTL direction changes

Maybe just my setup.
Comment 2 Jan-Marek Glogowski 2021-09-09 15:43:17 UTC
There is https://gerrit.libreoffice.org/c/core/+/121858
Comment 3 Jan-Marek Glogowski 2021-09-09 17:06:59 UTC
I just thought I could drop the whole nModMask handling for the nExtModMask info and just use the Qt modifier. But the Qt docs already warn about broken handling: "This function cannot always be trusted. The user can confuse it by pressing both Shift keys simultaneously and releasing one of them, for example." (https://doc.qt.io/qt-5/qkeyevent.html#modifiers). Probably because Qt reports the combined / new mask on change, but if I just change L/R, it reports an empty mask.

At this point I'm not sure the whole L/R info should be used. LO's code in qt5 + gtk + gen has the same problem, because it removes the mask, if one of the L/R modifier keys is released. So while my patch fixes the problem, I now think the whole approach is broken; maybe LO should replace it by always reporting both L/R keys.

I thought the Gtk approach might be good, until I found the behavior in comment 1.

And the Windows code looks funny too.

Looking for more opinions.
Comment 4 Michael Weghorn 2021-09-10 07:09:51 UTC
Reproduced with

Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: 5b6fcbf4a754a2699b3b865256848238f93113a2
CPU threads: 12; OS: Linux 5.10; UI render: default; VCL: kf5 (cairo+xcb)
Locale: en-GB (en_GB.UTF-8); UI: en-US
Calc: threaded

Note that the Shift key on the right-hand side has to be used, otherwise this does not happen. (It doesn't matter which Ctrl key is used.)

Easier way to get the same behaviour for both, kf5 and gtk3: Just press Ctrl+Shift (right-hand side Shift key) and paragraph is switched to RTL.
Comment 5 Michael Weghorn 2021-09-10 07:58:48 UTC
FWIW, some notes on trying to understand what's happening (without saying whether or not it should stay that way...):

The relevant code in SwEditWin::Command responsible for changing writing direction was added in commit

commit cf308f6beb1ca7d7d5e209bfc25d3094a7acd836
Author: Oliver Specht <os@openoffice.org>
Date:   Wed Nov 27 08:12:37 2002 +0000

    #105224# support COMMAND_MODKEYCHANGE to change writing direction


Unfortunately, #105224# refers to the StarDivision bug tracker.

However, looking at that commit, it looks a bit like it may be supposed to be a feature to switch to RTL by pressing Ctrl+Right_Shift and to LTR by pressing Ctrl+Left_Shift, at least when no other keys are pressed in addition?

For gtk3, I actually only get the switch when not pressing another key in addition (i.e. the Ctrl+Shift_Right+P case described here does not trigger the change in writing direction), while it's different for kf5.

With the change from comment 2, switching between RTL and LTR also no longer happens for kf5 when no other key is pressed in addition.
Comment 6 Caolán McNamara 2021-09-10 08:38:23 UTC
FWIW there's some older discussion of the ctrl+shift rtl/ltr feature at https://bugs.documentfoundation.org/show_bug.cgi?id=103158

And the MSOffice equivalent docs of
https://docs.microsoft.com/en-us/archive/blogs/office_global_experience/useful-tricks-and-shortcuts-for-users-around-the-world also has "Ctrl + the right Shift key changes Text direction to Right-to-Left. Ctrl + the left Shift key changes Text direction back to Left-to-Right."
Comment 7 Jan-Marek Glogowski 2021-09-10 10:47:55 UTC
Thanks for your comments!

I now fixed my patch reading the gtk3 code w.r.t. pThis->m_nKeyModifiers handling again and added the missed reset on normal key input, which seems to fix it.

And I added some (correct?) comments explaining the feature and implementation to the qt5 code.
Comment 8 Commit Notification 2021-09-10 16:57:27 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/61cb81d67ebf6b342a1cdb46bf6eb25a49eb5ff4

tdf#144412 Qt5 reset m_nModKeyCode on key input

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2021-09-13 05:15:21 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/8da3a97e715be1364fa8ba0bc5d73e231480e76c

tdf#144412 Qt5 reset m_nModKeyCode on key input

It will be available in 7.2.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.