Bug 96089 - Writer layout modifies document model, Autorecovery runs on unmodified file
Summary: Writer layout modifies document model, Autorecovery runs on unmodified file
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.1.0.0.alpha1
Hardware: All All
: medium normal
Assignee: Michael Stahl
URL:
Whiteboard: target:5.3.0 target:5.2.0.1 target:5.1.4
Keywords: bisected, regression
: 99590 (view as bug list)
Depends on:
Blocks: AutoSave
  Show dependency treegraph
 
Reported: 2015-11-26 15:29 UTC by Yousuf Philips (jay)
Modified: 2016-10-25 18:54 UTC (History)
8 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 Yousuf Philips (jay) 2015-11-26 15:29:06 UTC
Steps:
1) Open LO
2) Open Tools > Options > Load/Save > General and set autosave to 1 min
3) Open https://wiki.documentfoundation.org/images/0/0f/GS42-GettingStartedLO.odt
4) Autosave happens after 1 minute though the document wasnt modified
5) Once autosave completes, the undo split button is filled with 'change style' entries.

This doesnt happen in 5.0 daily.

Version: 5.1.0.0.alpha1+
Build ID: f6bc5b79c31225c02e9500d0ced4bd26f998f82b
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2015-11-24_01:06:28
Locale: en-US (en_US.UTF-8)
Comment 1 Buovjaga 2015-11-28 19:38:31 UTC
I only have "Save autorecovery.." and no autosave option.
Setting autorecovery to 1 min and restarting LibO does not give the effect you see with GS42-GettingStartedLO.odt

Win 7 Pro 64-bit Version: 5.2.0.0.alpha0+
Build ID: a7c3a2a9be83686657c06f37d521f9f6d2004ddd
Threads 4; Ver: Windows 6.1; Render: default; 
TinderBox: Win-x86@39, Branch:master, Time: 2015-11-28_04:39:18
Locale: fi-FI (fi_FI)
Comment 2 Jean-Baptiste Faure 2016-01-10 11:20:51 UTC
Autosave option has been made hidden since LO 5.0. Search autosave in expert configuration to change its value.
See bug 65509

Please, could you test with a clean new user profile?

Best regards. JBF
Comment 3 Yousuf Philips (jay) 2016-01-19 09:19:39 UTC
(In reply to Beluga from comment #1)
> I only have "Save autorecovery.." and no autosave option.

Yes autosave meant save autorecovery. :D
Comment 4 raal 2016-04-28 06:50:19 UTC
I can not confirm with Version: 5.2.0.0.alpha0+; win7. Jay, could you retest with actual version? Thanks
Comment 5 Yousuf Philips (jay) 2016-04-28 17:34:04 UTC
Still happening with cleared profile. Maybe someone should test it on Linux to see if its a Linux-only bug.

Version: 5.2.0.0.alpha0+
Build ID: b19ac3c4c6b4a41a1f3acac68b299fd676428a87
CPU Threads: 2; OS Version: Linux 4.2; UI Render: default; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2016-04-21_08:41:08
Locale: en-US (en_US.UTF-8)
Comment 6 Buovjaga 2016-04-29 10:06:58 UTC
Not reproduced.

Arch Linux 64-bit, KDE Plasma 5
Version: 5.2.0.0.alpha1+
Build ID: 29c4f7bd5863e34c449062aca6f8aee5ec7510a2
CPU Threads: 8; OS Version: Linux 4.5; UI Render: default; 
Locale: fi-FI (fi_FI.UTF-8)
Built on April 28th 2016
Comment 7 Yousuf Philips (jay) 2016-04-29 19:10:21 UTC
@stuart, @cor, @heiko: can any of you reproduce this?
Comment 8 Heiko Tietze 2016-04-29 21:30:00 UTC
After 4) nothing happens unless I agree with "Edit document" (notification panel at top since the file was downloaded from a suspicious place). After that 5) happens (undo change style). Not sure if this behavior is really unexpected.

Version: 5.1.2.2.0+
Build ID: 5.1.2.2 Arch Linux build-1
CPU Threads: 8; OS Version: Linux 4.5; UI Render: default; 
Locale: de-DE (de_DE.UTF8)
Comment 9 Heiko Tietze 2016-04-29 21:32:22 UTC
Addendum: It takes about 6 seconds on my system until the undo is enabled after opening the document. Guess this time is needed to parse through the entire document. Meaning also that I always have to agree to close without saving changes. Even after 10 seconds.
Comment 10 Buovjaga 2016-04-30 06:40:34 UTC
"Edit document" .. you opened it straight from the browser without downloading first. Try downloading fully and only then opening.
Comment 11 Heiko Tietze 2016-04-30 07:52:42 UTC
(In reply to Buovjaga from comment #10)
> Try downloading fully and only then opening.

Read my second comment.
Comment 12 Cor Nouws 2016-04-30 14:31:10 UTC
For me GettingStartedLO.odt more or less freezes with

Version: 5.2.0.0.alpha1+
Build ID: fe2bf7b05936bb3e84ccc5ddc3dad865a22de551
CPU Threads: 2; OS Version: Linux 4.4; UI Render: default; 
TinderBox: Linux-rpm_deb-x86@71-TDF, Branch:master, Time: 2016-04-27_00:55:13
Locale: nl-NL (nl_NL.UTF-8)

.. (not really, but working is not possible at all..)
Comment 13 Yousuf Philips (jay) 2016-04-30 18:18:21 UTC
Setting to new as Heiko confirmed the undo.

@Heiko: I also noticed that at one point even before the undo button became active, that if i closed the document it also showed save changes dialog.

@Cor: Yep my 2 cpu cores get hammered (> 90%) until it fully loads the 800+ page file, but luckily the UI doesnt freeze for me.
Comment 14 Michael Stahl 2016-06-08 14:40:36 UTC
the problem is caused by this call in SwDrawContact::Changed_()

      GetFmt()->GetDoc()->SetFlyFrmAttr( *(GetFmt()), aSet );

this is called during layout and changes the document model,
setting the modified status in the document.

it also creates the "Changed style" undo entries.

the AutoRecovery save happens because the document was set to modified.

this code is there since 2004, f830588605badb9c1b2e43567f4de558db7a601c
so it's not obvious why this bug would be a regression.

i'm very surprised that the layout does something like this, very odd...
Comment 15 Michael Stahl 2016-06-09 14:02:33 UTC
something about the number of pages after initial load; if you get 396 pages you never get a undo action, if you get ~580 or ~810 pages you get them

... bisected, regression from:

commit b4b7703e4335460cf48bfd6440f116359994c8ff
Author:     Noel Grandin <noelgrandin@gmail.com>
AuthorDate: Wed Oct 14 02:51:05 2015 +0200

    cppcheck:variableScope

... which causes the layout-cache that is stored in the document
to restore a lot more pages than 396.    

fixed on master

note that in general it's still possible that documents end up modified by the code mentioned in comment #14, that cannot be prevented currently
Comment 16 Commit Notification 2016-06-09 14:02:51 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96089 sw: fix scope of bBreakAfter in InsertCnt_()

It will be available in 5.3.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 17 Commit Notification 2016-06-09 19:04:52 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-5-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=905e38c7c6c02ec618b9231545c45debba3a8a44&h=libreoffice-5-2

tdf#96089 sw: fix scope of bBreakAfter in InsertCnt_()

It will be available in 5.2.0.1.

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 Commit Notification 2016-06-09 20:29:19 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=95ed42cc9f9cd40503ca609f1bcad31c5298889b&h=libreoffice-5-1

tdf#96089 sw: fix scope of bBreakAfter in InsertCnt_()

It will be available in 5.1.5.

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 19 Michael Stahl 2016-06-09 21:23:41 UTC
*** Bug 99590 has been marked as a duplicate of this bug. ***
Comment 20 Commit Notification 2016-06-10 13:30:22 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-5-1-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a73629cf36a484d58aba74057bacdf04d4728599&h=libreoffice-5-1-4

tdf#96089 sw: fix scope of bBreakAfter in InsertCnt_()

It will be available in 5.1.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.