Bug 88555 - FORMATTING: Undo and Redo With Custom Styles Crashes Writer
Summary: FORMATTING: Undo and Redo With Custom Styles Crashes Writer
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.4.0.2 rc
Hardware: All All
: highest critical
Assignee: Not Assigned
URL:
Whiteboard: target:5.4.0 target:5.3.4 target:5.3.3
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks: Undo-Draw
  Show dependency treegraph
 
Reported: 2015-01-18 13:36 UTC by yanjingtao
Modified: 2018-02-18 11:23 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
backtrace master 5.3 from Windbg.txt (9.82 KB, text/plain)
2016-09-19 15:20 UTC, Timur
Details
gdb backtrace (42.11 KB, text/plain)
2017-02-28 11:32 UTC, Xisco Faulí
Details

Note You need to log in before you can comment on or make changes to this bug.
Description yanjingtao 2015-01-18 13:36:46 UTC
When create a new style, and apply it to some words, then undo it (Ctrl+Z) all, and redo it(Ctrl+Y) all, you can find the style you create is lost, it maybe show in the list, but the style and formatting is lost, it become normal.


Steps to reproduce:
1. New Writer
2. Type something
3. Open Style and Formatting dialog
4. Create a new style
5. Apply this style to the words you type in step 2
6. Press Ctrl+Z until you can't do it
7. Press Ctrl+Y until you can't do it

You can find, the style you create has losted it's style and formatting.

OS: Windows 8.1
Version: 4.4.0.2
Build ID: a3603970151a6ae2596acd62b70112f4d376b990
Locale: zh_CN


This bug was first filed in the Chinese LibreOffice discussion forum:
http://www.libreofficechina.org/thread-817-1-1.html

somebody says in 4.3.2.1, the custom style is ok, but the words lost it's styles.
Comment 1 Cor Nouws 2015-01-18 14:17:15 UTC Comment hidden (obsolete)
Comment 2 yanjingtao 2015-01-19 01:18:11 UTC Comment hidden (obsolete)
Comment 3 Cor Nouws 2015-01-19 09:38:05 UTC Comment hidden (obsolete)
Comment 4 Kevin Suo 2015-01-19 13:27:13 UTC
Hi all,
I tested and find the following:

1. New Writer;

2. Create a new paragraph style (named "test") based on the default style, font color = blue.

3. Type in "abc" in your document, then apply style "test" to the text.

4. Undo until you can not undo. (I mean, undo all the steps, untill the "undo" toolbar icon is grayed out).

5. Redo untill you can not redo. (I mean, redo all the steps, untill the "redo" toolbar icon is grayed out).

--> "abc" is in black color, while blue is expected.

Aparently, by doing "undo" in step 4, 
* Firstly the applying of style "test" to text "abc" was undone;
* Then the style attributes of "test" was undone;
* Then the creation of style "test" was undone.

However, by doing "redo" in step 5, 
* The creation of style "test" was redone,
* But the style attributes of "test" was NOT redone (or was not redone correctly)
* Also the applying of style "test" to text "abc" was not redone.

Set to NEW
Version: 4.4.0.2
Build ID: a3603970151a6ae2596acd62b70112f4d376b990
Locale: zh_CN
Comment 5 Cor Nouws 2015-01-19 16:08:57 UTC
Now I can confirm this too.
Maybe because in my previous test I started with typing, and then the creation of a style.
Comment 6 Kevin Suo 2015-12-03 05:00:05 UTC
Bug still reproduciable with
Version: 5.0.4.1 (x64)
Build ID: 2def61bcbb29a7a8611b833682fe1291910b11ad
Locale: zh-CN (zh_CN)
Win10 x64
Comment 7 Phil Krylov 2016-02-07 12:46:52 UTC
Still reproducible with:
Version: 5.0.4.2
Build ID: 2b9802c1994aa0b7dc6079e128979269cf95bc78
Locale: en-US (en.UTF-8)
OS X 10.9
Comment 8 Phil Krylov 2016-02-17 09:35:16 UTC
Version: 5.1.0.3
Build ID: 5e3e00a007d9b3b6efb6797a8b8e57b51ab1f737
CPU Threads: 2; OS Version: -; UI Render: default; 
Locale: en-US (en.UTF-8)
OS X

In this version, the test from Comment #3 effectively crashes Writer, and a document recovery dialog appears.
Comment 9 Phil Krylov 2016-02-17 12:40:09 UTC
Crash backtrace:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x00000007a0008030
0x00007fff85e4bb22 in __dynamic_cast ()
(gdb) bt
#0  0x00007fff85e4bb22 in __dynamic_cast ()
#1  0x00000001157518a3 in SwUndoFormatAttr::IsFormatInDoc ()
#2  0x00000001157516c7 in SwUndoFormatAttr::UndoImpl ()
#3  0x000000011575e2a6 in SwUndo::RedoWithContext ()
#4  0x0000000101349915 in SfxUndoManager::ImplRedo ()
#5  0x000000011574af9a in sw::UndoManager::impl_DoUndoRedo ()
#6  0x000000011574b17e in sw::UndoManager::Redo ()
#7  0x000000011553a47c in SwEditShell::Redo ()
#8  0x0000000115c2f94e in SwWrtShell::Do ()
#9  0x0000000115b05e39 in SwBaseShell::ExecUndo ()
#10 0x0000000100d12408 in SfxDispatcher::Call_Impl ()
#11 0x0000000100d0d39c in SfxBindings::Execute_Impl ()
#12 0x0000000100d457fc in SfxDispatchController_Impl::dispatch ()
#13 0x0000000100d43f27 in SfxOfficeDispatch::dispatch ()
#14 0x0000000101f8e4a2 in svt::ToolboxController::execute ()
#15 0x0000000100f0a8e6 in SfxToolBoxControl::execute ()
#16 0x0000000111ebcd6e in framework::ToolBarManager::Select ()
#17 0x0000000102a8485e in ToolBox::Select ()
#18 0x0000000102a93622 in ToolBox::ImplHandleMouseButtonUp ()
#19 0x0000000102a94ad3 in ToolBox::Tracking ()
#20 0x0000000102a9ba54 in vcl::Window::EndTracking ()
#21 0x0000000102ab16aa in ImplHandleMouseEvent ()
#22 0x0000000102ab3397 in ImplWindowFrameProc ()
#23 0x0000000102d7ef84 in -[SalFrameView sendMouseEventToFrame:button:eventtype:] ()
#24 0x00007fff8d392145 in -[NSWindow sendEvent:] ()
#25 0x00007fff8d3335d4 in -[NSApplication sendEvent:] ()
#26 0x0000000102d758c7 in -[VCL_NSApplication sendEvent:] ()
#27 0x0000000102d411b6 in AquaSalInstance::DoYield ()
#28 0x0000000102cce29e in Application::Yield ()
#29 0x0000000102cce238 in Application::Execute ()
#30 0x000000010007c7a2 in desktop::Desktop::Main ()
#31 0x0000000102cd2472 in ImplSVMain ()
#32 0x0000000102d40be4 in AquaSalInstance::handleAppDefinedEvent ()
#33 0x0000000102d758a4 in -[VCL_NSApplication sendEvent:] ()
#34 0x00007fff8d1839f9 in -[NSApplication run] ()
#35 0x00007fff8d16e783 in NSApplicationMain ()
#36 0x0000000102d3fef9 in ImplSVMainHook ()
#37 0x0000000102cd309a in SVMain ()
#38 0x00000001000a71b9 in soffice_main ()
#39 0x0000000100000f20 in main ()
Comment 10 Timur 2016-09-19 15:20:24 UTC
Created attachment 127434 [details]
backtrace master 5.3 from Windbg.txt
Comment 11 Xisco Faulí 2017-02-28 11:18:48 UTC
Still reproducible in

Version: 5.4.0.0.alpha0+
Build ID: eb7b03b052ffe8c2c577b2349987653db6c53f76
CPU threads: 1; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86@62-merge-TDF, Branch:MASTER, Time: 2017-02-26_22:34:18
Locale: es-ES (es_ES); Calc: group

and

Version: 5.4.0.0.alpha0+
Build ID: bd8c68c99cce51a3368d8ddfd6e11dccb72d8f49
CPU threads: 4; OS: Linux 4.8; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 12 Xisco Faulí 2017-02-28 11:32:40 UTC
Created attachment 131532 [details]
gdb backtrace
Comment 13 Xisco Faulí 2017-02-28 11:42:02 UTC
Crash introduced by:

author	Katarina Behrens <Katarina.Behrens@cib.de>	2015-05-03 21:25:52 (GMT)
committer	Katarina Behrens <Katarina.Behrens@cib.de>	2015-05-03 21:31:53 (GMT)
commit 98436c4b53639d86f261ac630c46d32e3c7b8e28 (patch)
tree be3be3fb1cdd44ad92c8a171ef72e00a96815be2
parent bc9d02b0ca6244b46c9e2c59b7cc3618eb0f0148 (diff)
tdf#89783: Adjust to new GetPos retval (size_t vs. sal_uInt16)
Some of the usages of GetPos were just misusing it to find out
whether a vector contains given element -- use Contains() in those
cases

This patch is partially based on work of Christoph Lutz

Adding Cc: to Katarina Behrens
Comment 14 Katarina Behrens (Inactive) 2017-03-10 08:28:16 UTC
Okay guys, I can make the crash go away, but I can't fix non-functional redo. I lack deeper knowledge of Writer to actually do that ... shall I do at least that?
Comment 15 Timur 2017-03-10 08:51:48 UTC
Makes sense if crash is your regression from 5.0 and redo is some older problem from 4.4, like Bug 93045.
Comment 16 Xisco Faulí 2017-03-10 09:29:44 UTC
(In reply to Katarina Behrens (CIB) from comment #14)
> Okay guys, I can make the crash go away, but I can't fix non-functional
> redo. I lack deeper knowledge of Writer to actually do that ... shall I do
> at least that?

IMHO, we should make the crash go away despite of the fact the functionally doesn't work. Better to have a functionally not working than a crash
Comment 17 Commit Notification 2017-04-27 20:32:07 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1df637bde32c484b681ecdfebf56fdca03db7fc1

tdf#88555: band-aid fix, using GetPos/find instead of Contains

It will be available in 5.4.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 18 Michael Stahl (allotropia) 2017-04-28 21:00:15 UTC
regarding the crash, it is because commit 0f98299f7aa44bbb55c1bfeddca7799f727d14b0 inserted a dynamic_cast in the Contains function.

Undo of the style creation deletes the SwFormat:

#0  0x00007f08164adf1b in SwFormat::~SwFormat() (this=0x3d63770, __in_chrg=<optimized out>) at /work/lo/libreoffice-5-3/sw/source/core/attr/format.cxx:214
#1  0x00007f0816614874 in SwFormatColl::~SwFormatColl() (this=0x3d63770, __in_chrg=<optimized out>) at /work/lo/libreoffice-5-3/sw/inc/fmtcol.hxx:33
#2  0x00007f0816746f16 in SwTextFormatColl::~SwTextFormatColl() (this=0x3d63770, __in_chrg=<optimized out>) at /work/lo/libreoffice-5-3/sw/inc/fmtcol.hxx:54
#3  0x00007f0816746f32 in SwTextFormatColl::~SwTextFormatColl() (this=0x3d63770, __in_chrg=<optimized out>) at /work/lo/libreoffice-5-3/sw/inc/fmtcol.hxx:54
#4  0x00007f081660e8f5 in SwDoc::DelTextFormatColl(unsigned long, bool) (this=0x36cb7e0, nFormatColl=7, bBroadcast=true) at /work/lo/libreoffice-5-3/sw/source/core/doc/docfmt.cxx:991
#5  0x00007f081660ea11 in SwDoc::DelTextFormatColl(SwTextFormatColl*, bool) (this=0x36cb7e0, pColl=0x3d63770, bBroadcast=true) at /work/lo/libreoffice-5-3/sw/source/core/doc/docfmt.cxx:999
#6  0x00007f0816c98836 in SwUndoTextFormatCollCreate::Delete() (this=0x3d4f130) at /work/lo/libreoffice-5-3/sw/source/core/undo/SwUndoFmt.cxx:206
#7  0x00007f0816c97f77 in SwUndoFormatCreate::UndoImpl(sw::UndoRedoContext&) (this=0x3d4f130) at /work/lo/libreoffice-5-3/sw/source/core/undo/SwUndoFmt.cxx:63

the call to SwFormatsModifyBase::Contains effectively wants to check if
the SwFormat has been deleted; i think every SwFormat is in some SwDoc
member array for as long as it is alive.

of course with the dynamic_cast in Contains, the check if it's
still alive turns into use-after-free crash.

the patch in commit #17 should fix this crash but every other
call to Contains probably either crashes or is pointless because
it will always return true.
Comment 19 Commit Notification 2017-05-02 12:57:03 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e7068a7d9b945e0c6d4965445b6d951038e9c987&h=libreoffice-5-3

tdf#88555: band-aid fix, using GetPos/find instead of Contains

It will be available in 5.3.4.

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

Affected users are encouraged to test the fix and report feedback.
Comment 20 Commit Notification 2017-05-03 11:19:48 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-3-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d866bb51927b8b75718438a63138834bb7ea2f6c&h=libreoffice-5-3-3

tdf#88555: band-aid fix, using GetPos/find instead of Contains

It will be available in 5.3.3.

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

Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2017-05-03 12:11:57 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=01575a06725648188d51de90323a6f1da97ef7a9

tdf#88555 sw: remove dynamic_cast from SwFrameFormats::Contains

It will be available in 5.4.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 22 Commit Notification 2017-05-03 12:12:35 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8d31f114327e77c48c8cdc804e1e399ebeadd27c

tdf#88555 sw: reduce usage of dynamic_cast in SwFormatsModifyBase::Contains

It will be available in 5.4.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2017-05-03 12:13:10 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=91646b10315e2ee64b25fb8a604012c729343c07

tdf#88555 sw: use safe IsAlive function in Undo code

It will be available in 5.4.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 24 Timur 2017-05-23 10:08:19 UTC
Not easy to reproduce but I did per Comment 4, crash in 5.2 and formatting lost in 5.5.+. So, if crash was fixed, styles was not.
Comment 25 Kevin Suo 2017-11-04 10:45:00 UTC
The crash issue is fixed. I split the lost of formatting to new bug report (bug 113640).
Comment 26 Xisco Faulí 2017-11-04 16:24:38 UTC
(In reply to Kevin Suo from comment #25)
> The crash issue is fixed. I split the lost of formatting to new bug report
> (bug 113640).

Thanks Kevin!
Comment 27 Xisco Faulí 2017-11-08 08:01:05 UTC
setting to VERIFIED as per comment 25