Bug 149485 - "Show Whitespace" condition changes other documents look
Summary: "Show Whitespace" condition changes other documents look
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.4.0.0 alpha0+
Hardware: All All
: high normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.6.0 target:7.5.3 target:7.4.7
Keywords: filter:odt, implementationError
Depends on:
Blocks: Show/Hide-Whitespace
  Show dependency treegraph
 
Reported: 2022-06-08 07:55 UTC by NISZ LibreOffice Team
Modified: 2023-04-27 03:52 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Test file which show whitespace (You can see the margins.) (8.39 KB, application/vnd.oasis.opendocument.text)
2022-06-08 07:58 UTC, NISZ LibreOffice Team
Details
Comparing the files look (60.10 KB, image/png)
2022-06-08 07:59 UTC, NISZ LibreOffice Team
Details

Note You need to log in before you can comment on or make changes to this bug.
Description NISZ LibreOffice Team 2022-06-08 07:55:33 UTC
Description:
When I create a writer document and turn on the "Show Whitespace" and save it (or open a document what saved with this setup) after that I open the attached file which is without whitespace option.
The last opened document looks like when "Show Whitespace" condition of turned on.

Steps to Reproduce:
1. Create a new Writer document
2. Write something in it.
3. Turn on the Whitespace option. View -> Show Whitespace
4. Save it.
5. Close it.
5. Open the attached document which does not contain this setup (without whitespace).

Actual Results:
The attached document which is not contain "Show Whitespace" condition looks like as if it is. The previous document looks like change this setup on other documents in LO.

Expected Results:
The document should look like when it first opened.


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.4.0.0.alpha1+ (x64) / LibreOffice Community
Build ID: f572585756494c59fb81f5d93c51cc2d35421f0e
CPU threads: 8; OS: Windows 10.0 Build 19042; UI render: Skia/Vulkan; VCL: win
Locale: hu-HU (hu_HU); UI: en-US
Calc: CL
Comment 1 NISZ LibreOffice Team 2022-06-08 07:58:09 UTC
Created attachment 180628 [details]
Test file which show whitespace (You can see the margins.)
Comment 2 NISZ LibreOffice Team 2022-06-08 07:59:12 UTC
Created attachment 180629 [details]
Comparing the files look
Comment 3 Stéphane Guillou (stragu) 2023-03-20 14:48:01 UTC
I was about to report the same thing.

This is since:

commit 5b07acbf3345918f450fccf7ee243ad5bcb3fd67
author	Dhiraj Holden <dhiraj.holden@gmail.com>	Tue Dec 07 08:16:59 2021 -0500
committer	Mike Kaganski <mike.kaganski@collabora.com>	Mon Jan 17 07:33:18 2022 +0100
tdf#142450 add code to store showing whitespace
I have put in code to store the option to show whitespace.
This option is stored at the document level like the other
layout options.
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126497

I would call it an implementation error.

Now, if your preferred setting is to show whitespace (the default setting when starting with a fresh profile), it will be overwritten if you happen to open someone's document that has "show whitespace" off. Meaning all further Writer sessions will start with whitespace off.
And vice-versa.

So it has two consequences:
- the in-document setting overwrites the user default for new documents
- the in-document setting will "leak" into existing documents if they don't have that property set already (i.e. documents created before LO 7.4)

In my opinion, this new feature should better differentiate between (1) the imported document property and (2) the setting changed by user action.

Marking as "high" priority because it silently modifies existing documents.

Heiko and Mike, adding you as you participated in the discussion.
Comment 4 Heiko Tietze 2023-03-21 08:42:38 UTC
Mike's argument in bug 142450 comment 15 was to align the white space with single-page layout (to not interfere if one is on and the other off), and to save the option therefor with the document rather than on the workplace.

As a compromise I could imagine we check both similar to the last position when the document was saved. Plus, we must not overwrite the default that is used for new documents; the option could become a tristate if read and changed from document.

In the end it's cumbersome to save the option with the document. Perhaps Mike can change his mind.
Comment 5 Mike Kaganski 2023-03-21 08:47:19 UTC
(In reply to Heiko Tietze from comment #4)

I'm sorry, but as far as I can tell, this bug report is in complete agreement with my argument there: it tells that it is *wrong* when you define the option for one document, but it affects other documents! I told the same, telling that we should only limit the effect of the option to one specific document where it's configured and stored.

So this report only tells that this idea (limiting the effect to only one document) was not implemented correctly.
Comment 6 Xisco Faulí 2023-03-23 15:49:32 UTC
(In reply to Mike Kaganski from comment #5)
> (In reply to Heiko Tietze from comment #4)
> 
> I'm sorry, but as far as I can tell, this bug report is in complete
> agreement with my argument there: it tells that it is *wrong* when you
> define the option for one document, but it affects other documents! I told
> the same, telling that we should only limit the effect of the option to one
> specific document where it's configured and stored.
> 
> So this report only tells that this idea (limiting the effect to only one
> document) was not implemented correctly.

Hi Mike,
Do you think the commit should be reverted ?
Comment 7 Mike Kaganski 2023-03-23 16:11:28 UTC
(In reply to Xisco Faulí from comment #6)

It depends on the severity of the problem. Ideally, it would be best if it is investigated and fixed; possibly, Dhiraj Holden could take a look?
Comment 8 Dhiraj Holden 2023-03-23 17:58:10 UTC
I'll see if I can take a look but I'm busy and don't have the build set up on my computer right now.
Comment 9 Xisco Faulí 2023-03-24 11:45:12 UTC
(In reply to Dhiraj Holden from comment #8)
> I'll see if I can take a look but I'm busy and don't have the build set up
> on my computer right now.

Ok, no problem.
I decided to revert the patch for now so it can be backported to previous branches -> https://gerrit.libreoffice.org/c/core/+/149541
Anyway, it would be great if you could take a look at it whenever you have some time. Thanks in advance
Comment 10 Commit Notification 2023-03-24 14:12:13 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

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

tdf#149485: Revert "tdf#142450 add code to store showing whitespace"

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 11 Commit Notification 2023-03-25 19:42:09 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

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

tdf#149485: Revert "tdf#142450 add code to store showing whitespace"

It will be available in 7.5.3.

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 12 Commit Notification 2023-03-26 10:11:30 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "libreoffice-7-4":

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

tdf#149485: Revert "tdf#142450 add code to store showing whitespace"

It will be available in 7.4.7.

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.