Bug 143778 - incorrect detection whether document changed
Summary: incorrect detection whether document changed
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.4.0.3 release
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.3.0 target:7.2.1 target:7.1.6
Keywords: bibisected, bisected, regression
Depends on:
Blocks: ModifiedStatus
  Show dependency treegraph
 
Reported: 2021-08-08 13:54 UTC by csongor
Modified: 2021-08-18 15:11 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Cancelling the Shape -> Area dialog is detected as document changed (9.80 KB, application/vnd.oasis.opendocument.text)
2021-08-08 13:54 UTC, csongor
Details

Note You need to log in before you can comment on or make changes to this bug.
Description csongor 2021-08-08 13:54:19 UTC
Created attachment 174144 [details]
Cancelling the Shape -> Area dialog is detected as document changed

Here are the steps to reproduce:

- File -> New -> Text Document
- Insert -> Shape -> Basic Shapes -> Rectangle -> Draw a rectangle anywhere on the page
- save the document
- exit Writer
- Open the file in Writer again
- Right click the shape
- Select "Area"
- don't change anything, just click "Cancel" 
- Close the document

What happens:
A dialog appears: "Save changes to document “sample.odt” before closing?"

What I expect:
Close the document without asking because there was no change to save.
Comment 1 NISZ LibreOffice Team 2021-08-09 08:12:25 UTC
Confirming with

Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: d1f1f546b212ecd651146addeb328806bb270d5f
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win
Locale: en-US (hu_HU); UI: hu-HU
Calc: CL
Comment 2 NISZ LibreOffice Team 2021-08-09 09:19:40 UTC
This was good until 6.3, started in 6.4:

https://git.libreoffice.org/core/+/b82fdec369449e87df24acc8fa0daa54f2aeb4da

author	Muhammet Kara <muhammet.kara@collabora.com>	Thu Jun 27 18:06:06 2019 +0300
committer	Muhammet Kara <muhammet.kara@collabora.com>	Thu Jun 27 23:38:01 2019 +0200

lokdialog: Convert the Format -> ... -> Area... to async exec.

Adding CC to: Muhammet Kara
Comment 3 NISZ LibreOffice Team 2021-08-09 10:11:59 UTC
Note: opening & closing the Position and Size dialog also sets modified status, but that's an even earlier regression. I'll file that separately.
Comment 4 Caolán McNamara 2021-08-16 14:37:44 UTC
The start of SwDrawShell::ExecDrawDlg has...

bool bChanged = pDoc->IsChanged();
pDoc->SetChanged(false);

and the end has

if (pDoc->IsChanged())
    GetShell().SetModified();
else
    if (bChanged)
        pDoc->SetChanged();

and before async dialogs the start and end happened before and after the dialog appeared and disappeared. The intent seems to be unset the doc-changed and restore its original state if the dialogs caused nothing to happen and to explicitly set SetModified if something did.

Now the async dialogs callback happens after SwDrawShell::ExecDrawDlg has ended and so the callbacks start with SdrModel::IsChanged at its original value (restored by the end of ExecDrawDlg), not the "false" they were originally written to expect
Comment 5 Commit Notification 2021-08-17 07:36:49 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

tdf#143778 these async callbacks expect to have SdrModel::IsChanged of false

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 6 Caolán McNamara 2021-08-17 07:39:11 UTC
fixed in trunk, backport to 7-2 and 7-1 in gerrit
Comment 7 Commit Notification 2021-08-17 13:15:50 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/79b138b64a49467e63c1ec413fabf06c18c3fede

tdf#143778 these async callbacks expect to have SdrModel::IsChanged of false

It will be available in 7.2.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 8 Commit Notification 2021-08-17 13:17:05 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/6fc0b5ce07eee825d6f8a2741dfe1fa7a9238bea

tdf#143778 these async callbacks expect to have SdrModel::IsChanged of false

It will be available in 7.1.6.

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-08-18 15:11:48 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4297f40c6ab03701ffebf670cbf1186573ba6a15

tdf#143778, tdf#143785: sw: add UItest

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.