Bug 152404 - Crash in Writer when using Japanese Hiragana input method and shortcut Ctrl+Alt+C to insert a new comment while there is uncommitted text
Summary: Crash in Writer when using Japanese Hiragana input method and shortcut Ctrl+A...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: All Windows (All)
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.6.0 target:7.5.1 target:7.4.6
Keywords: haveBacktrace
Depends on:
Blocks: CJK Shortcuts-Accelerators
  Show dependency treegraph
 
Reported: 2022-12-07 00:16 UTC by Patrick (volunteer)
Modified: 2023-02-02 11:30 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Backtrace (1.81 KB, text/plain)
2022-12-23 16:07 UTC, Aron Budea
Details
bt with debug symbols (3.93 KB, text/plain)
2023-01-22 10:59 UTC, Julien Nabet
Details
quick demo plausible fix (585 bytes, patch)
2023-01-22 12:34 UTC, Caolán McNamara
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick (volunteer) 2022-12-07 00:16:08 UTC
Description:
After some debugging, I found that the crash occurs when the new comment grabs focus, it invokes WinSalFrame::EndExtTextInput() which is supposed to commit the uncommitted text. Unfortunately, Writer's input method handler commits the text in two steps: first it deletes the uncommitted text, then it inserts the committed text.

The crash occurs in the first step: when Writer deletes the uncommitted text, it triggers Writer to remove the newly added comment and the comment's vcl objects are all deleted.

Steps to Reproduce:
Steps to reproduce. Note: this crash occurs in Windows but not macOS since macOS has not yet implemented SalFrame::EndExtTextInput():

1. Open a new Writer document
2. Change your keyboard entry to a Chinese, Japanese, or Korean input method (I used Japanese Hiragana)
3. Type a few characters so that the text is in an uncommitted state (I typed "aaa" on a US English physical keyboard which get converted to "あああ" by the Japanese Hiragana input method)
4. Press the Control-Alt-C keys to create a new comment
5. Crash

Actual Results:
Application crashes due to use of a deleted pointer.

Expected Results:
Uncommitted text should be committed and the cursor moved to the new comment.


Reproducible: Always


User Profile Reset: Yes

Additional Info:
Version: 7.5.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: c08e5db055c9d34d3f0b0b9d2a192d7ebdcd9576
CPU threads: 1; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded
Comment 1 Patrick (volunteer) 2022-12-07 00:25:04 UTC
One more detail: if, in step 4, you select the Insert > Comment menu item, the crash does not occur and uncommitted text is committed and the cursor moves to the new comment.

So, it appears that selecting a menu item commits the text before the new comment is created but pressing a shortcut that contains the Control key bypasses the commit process.
Comment 2 Dieter 2022-12-23 09:46:23 UTC
Thank you for reporting the bug. I can't test it, because I can't change to Japanese keyboard. It looks like a problem with shortcut and i wonder if Ctrl+Alt+C is the only combination that causes a crash
Comment 3 Aron Budea 2022-12-23 16:07:37 UTC
Confirmed using LO 7.6.0.0.alpha0+ (40db42be1d8fd0f9c6c8c5ba3767ddb9ee2034c2), 4.0.0.3 / Windows.
Comment 4 Aron Budea 2022-12-23 16:07:59 UTC
Created attachment 184331 [details]
Backtrace
Comment 5 Patrick (volunteer) 2022-12-23 16:30:49 UTC
Note: this bug has been already fixed in macOS in the following patch:

https://cgit.freedesktop.org/libreoffice/core/commit/vcl/osx/vclnsapp.mm?id=9ee57f36e26373ee7144d076c93c3462c4fc7110

The above patch essentially calls SalFrame::EndExtTextInput(EndExtTextInputFlags::Complete) just before dispatching any key events with the Command key pressed.

I assume that a similar fix could be used for Windows and Linux by making the same call just before dispatching key events with the Control key pressed.
Comment 6 Julien Nabet 2023-01-22 10:59:30 UTC
Created attachment 184828 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this with gtk3 rendering.
(I'm gonna give a try with gen rendering).
Comment 7 Julien Nabet 2023-01-22 11:14:45 UTC
I got only a crash with gtk3, I don't reproduce this with gen or kf5 rendering even if behavior is different between both.

I'm on pc Debian x86-64 with master sources updated today + enable-dbgutil and use ibus-mozc with Japanese Hiragana.

With this patch:
diff --git a/vcl/source/window/mouse.cxx b/vcl/source/window/mouse.cxx
index fe9e2b0f2444..db7fb835453e 100644
--- a/vcl/source/window/mouse.cxx
+++ b/vcl/source/window/mouse.cxx
@@ -292,7 +292,7 @@ void Window::ImplGrabFocus( GetFocusFlags nFlags )
 
     // mark this windows as the last FocusWindow
     vcl::Window* pOverlapWindow = ImplGetFirstOverlapWindow();
-    if (pOverlapWindow->mpWindowImpl)
+    if (pOverlapWindow && pOverlapWindow->mpWindowImpl)
         pOverlapWindow->mpWindowImpl->mpLastFocusWindow = this;
     mpWindowImpl->mpFrameData->mpFocusWin = this;
 

I got:
#0  0x00007fdc3d4d880b in vcl::Window::ImplGrabFocus(GetFocusFlags) (this=0x555e7d18b270, nFlags=GetFocusFlags::NONE) at vcl/source/window/mouse.cxx:297
#1  0x00007fdc3d5a6de7 in vcl::Window::GrabFocus() (this=0x555e7d18b270) at vcl/source/window/window.cxx:2985
#2  0x00007fdc17982a15 in SwPostItMgr::LayoutPostIts() (this=0x555e7cbc0e50) at sw/source/uibase/docvw/PostItMgr.cxx:796
297	    mpWindowImpl->mpFrameData->mpFocusWin = this;
(gdb) p mpWindowImpl
$1 = std::unique_ptr<WindowImpl> = {get() = 0x0}


Concerning EndExtTextInput, I added a trace just before:
    288     // EndExtTextInput if it is not the same window
    289     if (pSVData->mpWinData->mpExtTextInputWin
    290         && (pSVData->mpWinData->mpExtTextInputWin.get() != this))
    291         pSVData->mpWinData->mpExtTextInputWin->EndExtTextInput();

and the line 291 is indeed called.

Caolán: any idea here?
perhaps the fix for gtk3 may help for Windows or at least provide some hints.
Comment 8 Caolán McNamara 2023-01-22 12:34:34 UTC
Created attachment 184831 [details]
quick demo plausible fix

I can't reproduce with my Japanese Input Method, which seems to flush itself on any ctrl+alt+foo combination before getting to LibreOffice. Nevertheless does this demo patch avoid the crash for the gtk3 case Julien can reproduce?
Comment 9 Patrick (volunteer) 2023-01-22 14:04:40 UTC
(In reply to Caolán McNamara from comment #8)
> Created attachment 184831 [details]
> quick demo plausible fix
> 

One question about your patch: the macOS code dispatches an EndExtTextInputFlags::Complete event and commits any uncommitted text when the Command key is pressed. On both macOS and Windows, selecting a menu item also commits any uncommitted text.

Without your patch, does Ctrl-Alt-foo commit or discard uncommitted text?
Comment 10 Caolán McNamara 2023-01-22 14:44:13 UTC
"Without your patch, does Ctrl-Alt-foo commit or discard uncommitted text?" In my case (out of the box F38 GNOME with whatever is the default IM I get from setting, keyboard, input sources) it commits the preedit text and LibreOffice doesn't receive the "foo".
Comment 11 Patrick (volunteer) 2023-01-22 15:00:52 UTC
(In reply to Caolán McNamara from comment #10)
> "Without your patch, does Ctrl-Alt-foo commit or discard uncommitted text?"
> In my case (out of the box F38 GNOME with whatever is the default IM I get
> from setting, keyboard, input sources) it commits the preedit text and
> LibreOffice doesn't receive the "foo".

So it commits the text and the Ctrl-Alt-foo is ignored. On macOS, the Command-Option-foo does get dispatched after the text is committed. If you change your patch to use EndExtTextInputFlags::Complete, does it still longer ignore the Ctrl-Alt-foo shortcut?
Comment 12 Caolán McNamara 2023-01-23 09:17:25 UTC
"If you change your patch to use EndExtTextInputFlags::Complete, does it still longer ignore the Ctrl-Alt-foo shortcut?" My patch does nothing for me, ctrl+alt+whatever doesn't get delivered to libreoffice for me in either case, presumably the IM has consumed it. But I wonder if it does do something for those who can reproduce this under gtk.
Comment 13 Julien Nabet 2023-01-23 11:14:00 UTC
(In reply to Caolán McNamara from comment #12)
> "If you change your patch to use EndExtTextInputFlags::Complete, does it
> still longer ignore the Ctrl-Alt-foo shortcut?" My patch does nothing for
> me, ctrl+alt+whatever doesn't get delivered to libreoffice for me in either
> case, presumably the IM has consumed it. But I wonder if it does do
> something for those who can reproduce this under gtk.

with:
diff --git a/vcl/unx/gtk3/gtkframe.cxx b/vcl/unx/gtk3/gtkframe.cxx
index e9127b23e659..0450e875330b 100644
--- a/vcl/unx/gtk3/gtkframe.cxx
+++ b/vcl/unx/gtk3/gtkframe.cxx
@@ -451,6 +451,9 @@ bool GtkSalFrame::doKeyCallback( guint state,
     bool bStopProcessingKey;
     if (bDown)
     {
+        if (aEvent.mnCode | KEY_MOD1) // tdf#152404
+            EndExtTextInput(EndExtTextInputFlags::Complete);
+
         bStopProcessingKey = CallCallbackExc(SalEvent::KeyInput, &aEvent);
         // #i46889# copy AlternateKeyCode handling from generic plugin
         if (!bStopProcessingKey)


I got exactly the same crash:
#0  0x00007f2bb54b2c85 in std::__uniq_ptr_impl<WindowImpl, std::default_delete<WindowImpl> >::_M_ptr() const (this=0x8)
    at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:191
#1  0x00007f2bb54b2c65 in std::unique_ptr<WindowImpl, std::default_delete<WindowImpl> >::get() const (this=0x8) at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:462
#2  0x00007f2bb54f7595 in std::unique_ptr<WindowImpl, std::default_delete<WindowImpl> >::operator bool() const (this=0x8)
    at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:479
#3  0x00007f2bb56d87a9 in vcl::Window::ImplGrabFocus(GetFocusFlags) (this=0x55a686cd7140, nFlags=GetFocusFlags::NONE) at vcl/source/window/mouse.cxx:295
#4  0x00007f2bb57a6de7 in vcl::Window::GrabFocus() (this=0x55a686cd7140) at vcl/source/window/window.cxx:2985
#5  0x00007f2b8f982075 in SwPostItMgr::LayoutPostIts() (this=0x55a68588d080) at sw/source/uibase/docvw/PostItMgr.cxx:796
#6  0x00007f2b8f98ce5f in SwPostItMgr::CalcHdl(void*) (this=0x55a68588d080) at sw/source/uibase/docvw/PostItMgr.cxx:2243
#7  0x00007f2b8f97c08d in SwPostItMgr::LinkStubCalcHdl(void*, void*) (instance=0x55a68588d080, data=0x0) at sw/source/uibase/docvw/PostItMgr.cxx:2229
Comment 14 Caolán McNamara 2023-01-24 10:17:43 UTC
I can reproduce under windows however. In that case https://gerrit.libreoffice.org/c/core/+/146064 seems to work there to avoid the crash
Comment 15 Caolán McNamara 2023-01-24 12:09:02 UTC
I can reproduce under Linux after

dnf install ibus-mozc
ibus restart
and selecting the mozc input method, and click to dismiss the input method menu/window and then ctrl+alt+c
Comment 16 Commit Notification 2023-01-24 12:33:11 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/668f55b65849012b359d7d6b9386113facbadc57

tdf#152404 crash in Windows with ctrl+alt+c during ExtTextInput

It will be available in 7.6.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 17 Commit Notification 2023-01-24 13:34:25 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/d860ab4279afb9864da4979082eadcd8a24c4ebc

tdf#152404 crash in Windows with ctrl+alt+c during ExtTextInput

It will be available in 7.5.1.

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 18 Julien Nabet 2023-01-24 20:24:18 UTC
Just for the record, I gave a new try with the patch, I still got the same crash:
#3  0x00007f93c70d8be9 in vcl::Window::ImplGrabFocus(GetFocusFlags) (this=0x55f5b9038fe0, nFlags=GetFocusFlags::NONE) at vcl/source/window/mouse.cxx:295
#4  0x00007f93c71a7227 in vcl::Window::GrabFocus() (this=0x55f5b9038fe0) at vcl/source/window/window.cxx:2985
#5  0x00007f93979836a5 in SwPostItMgr::LayoutPostIts() (this=0x55f5b6ae35f0) at sw/source/uibase/docvw/PostItMgr.cxx:796
#6  0x00007f939798e48f in SwPostItMgr::CalcHdl(void*) (this=0x55f5b6ae35f0) at sw/source/uibase/docvw/PostItMgr.cxx:2243
#7  0x00007f939797d6bd in SwPostItMgr::LinkStubCalcHdl(void*, void*) (instance=0x55f5b6ae35f0, data=0x0) at sw/source/uibase/docvw/PostItMgr.cxx:2229

Just to be sure, I did:
make vcl.clean && make && make postprocess + remove LO profile to generate a brand new one.
Comment 19 Commit Notification 2023-01-24 21:17:15 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d8b60e66bf1847989f6ff3c06ea214cef059ecaf

tdf#152404 crash with ibus-mozc with ctrl+alt+c during ExtTextInput

It will be available in 7.6.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 20 Julien Nabet 2023-01-25 07:55:57 UTC
On pc Debian x86-64 with master sources updated today, I don't reproduce the crash anymore!
After having typed "あああ", when doing Ctrl-Alt-C, LO lets あああ in the document and an empty comment is created.

Thank you Caolán!
Comment 21 Caolán McNamara 2023-01-25 09:46:42 UTC
lets call that fixed then, various backports in gerrit.
Comment 22 Patrick (volunteer) 2023-01-25 21:14:49 UTC
(In reply to Julien Nabet from comment #20)
> On pc Debian x86-64 with master sources updated today, I don't reproduce the
> crash anymore!
> After having typed "あああ", when doing Ctrl-Alt-C, LO lets あああ in the document
> and an empty comment is created.
> 

I just downloaded and tested the Windows nightly build on Windows 10 and I now get the same results as @Julien Nabet. So, marking as Verified.
Comment 23 Commit Notification 2023-01-27 10:38:18 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-4":

https://git.libreoffice.org/core/commit/2abb6c0df76514d57169ea0600c7f48ec543789a

tdf#152404 crash in Windows with ctrl+alt+c during ExtTextInput

It will be available in 7.4.5.

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 24 Xisco Faulí 2023-01-27 14:08:03 UTC
7.4.5 was a hotfix release, updating target in status-whiteboard
Comment 25 Commit Notification 2023-02-02 11:30:12 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/c1b14f94ca800577eb79eec3cc69b764017a6e25

tdf#152404 crash with ibus-mozc with ctrl+alt+c during ExtTextInput

It will be available in 7.5.1.

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.