Bug 118845 - Both Male and Female Salutations shown when hide paragraphs of empty fields option selected
Summary: Both Male and Female Salutations shown when hide paragraphs of empty fields o...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:6.2.0 target:6.1.2
Keywords:
Depends on:
Blocks: Mail-Merge
  Show dependency treegraph
 
Reported: 2018-07-19 16:24 UTC by Dom Walden
Modified: 2018-09-01 23:04 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
When "Hide paragraphs..." option is ticked (2.88 KB, image/png)
2018-07-19 16:27 UTC, Dom Walden
Details
When "Hide paragraphs..." option is NOT ticked (2.43 KB, image/png)
2018-07-19 16:27 UTC, Dom Walden
Details
Example address list for use in reproduction (10.64 KB, application/vnd.oasis.opendocument.spreadsheet)
2018-07-19 16:28 UTC, Dom Walden
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dom Walden 2018-07-19 16:24:46 UTC
Description:
If I have a mail merge document with a "personalized salutation" (male and female) and I have the compatibility option "Hide paragraphs of database fields (e.g., mail merge) with an empty value" ticked, when I "Edit Individual Documents" or "Save Merged Documents" the resulting documents contain both the male and female salutation. I will attach screenshot example.

This may also be true when I Print or Email the merged documents, but I have not tried this.

If I untick the "Hide paragraphs..." option, only the one line of salutation (for either male or female) will show. I will attach screenshot example.

I have reproduced this on 6.1RC1 and today's nightly build.

Steps to Reproduce:
1. Open a new Write document.
2. Go to Tools > Options > LibreOffice Writer > Compatibility and check that "Hide paragraphs of database fields..." is ticked. Tick it if not and click OK.
3. Select Tools > Mail Merge Wizard...
4. Check radio button "Use current document" and click Next.
5. Check radio button "E-mail message" (it is also reproducible with "Letter") and click Next.
6. If you do not have an address list, click Select Different Address List, in the new dialog click "Add..." and select an address list (you may use the one I will attach). Click OK and then Next.
7. Make sure that both "This document should contain a salutation" and "Insert personalized salutation" are ticked, and that you have both a Male and Female variant of the Salutation (which I guess is default). Click Next and then Finish.
8. Click "Edit Individual Documents"

Actual Results:
You will see one or more new documents which start:
"Dear Mrs. Lname,
Dear Mr. Lname,"

Expected Results:
Each document should start either:
"Dear Mrs. Lname,"
or
"Dear Mr. Lname,"
(In our case probably the latter as we have not distinguished the genders of our address entries)


Reproducible: Always


User Profile Reset: Yes



Additional Info:
_Versions_
6.1RC1 (6.1.0.1_Linux_x86-64_deb)
6.1.0.1.0+ 4e5248f32d8fdfd4655bd15bd60d83e9a0c6e540 2018-07-19_00:36:46 Linux-rpm_deb-x86_64

The option: "Hide paragraphs of database fields (e.g., mail merge) with an empty value" was added under bug 35798 and is listed in the 6.1 Release Notes https://wiki.documentfoundation.org/ReleaseNotes/6.1#Mail_merge.
Comment 1 Dom Walden 2018-07-19 16:27:27 UTC
Created attachment 143647 [details]
When "Hide paragraphs..." option is ticked
Comment 2 Dom Walden 2018-07-19 16:27:48 UTC
Created attachment 143648 [details]
When "Hide paragraphs..." option is NOT ticked
Comment 3 Dom Walden 2018-07-19 16:28:20 UTC
Created attachment 143649 [details]
Example address list for use in reproduction
Comment 4 Susan Gessing 2018-07-19 20:21:56 UTC
Used the attached address list and followed the steps. I was able to reproduce it in:

Version: 6.2.0.0.alpha0+
Build ID: b1740fba0d1e6e3d69c3781734509317f42a0e4f
CPU threads: 4; OS: Windows 6.3; UI render: GL; 
TinderBox: Win-x86@42, Branch:master, Time: 2018-06-15_08:49:04
Locale: en-US (en_US); Calc: CL
Comment 5 Xisco Faulí 2018-07-20 15:22:48 UTC
I guess this could be a good place to start with: https://opengrok.libreoffice.org/xref/core/sw/source/ui/dbui/mmlayoutpage.cxx#422
Comment 6 Michael Stahl (CIB) 2018-08-29 14:26:29 UTC
presumably caused by:

commit db04be037b611e296ef9f2542322c52ed82d7a2b
Author:     Mike Kaganski <mike.kaganski@collabora.com>
AuthorDate: Fri May 18 18:48:38 2018 +0300

    tdf#35798: Hide empty Database fields' paragraphs (+ compat option)
    
    With this change, Database fields that expand to empty values behave
    as if they are "Hidden Paragraph" fields.
    
    A compatibility option to enable this behaviour is added. The option is
    enabled by default, and for any non-native documents (for compatibility
    with other office suites). For existing (F)ODT documents, the option is
    disabled for those documents that don't have this setting set, to keep
    the layout of legacy documents.

Change in SwpHints::CalcHiddenParaField().

The issue is that the DB-field in the paragraph is non-empty and that now overrides the HiddenParagraph-field.
Comment 7 Michael Stahl (CIB) 2018-08-29 14:37:33 UTC
I don't like that CalcHiddenParaField treats both Database fields and HiddenParagraph fields the same.

For example, if we consider only Database fields, then the logic in the function looks appropriate: if there is one Database field in the paragraph that is non-empty, then the paragraph should not be hidden by database fields.

But as far as the HiddenParagraph field is concerned, i'm not so sure.

And the interaction of Database field and HiddenParagraph field should be such that if the HiddenParagraph fields decide to hide the paragraph, the Database fields should not override that decision.
Comment 8 Mike Kaganski 2018-08-30 09:31:03 UTC
(In reply to Michael Stahl from comment #7)

Thank you for analysis! You are quite right.
https://gerrit.libreoffice.org/59792 implements "levels" (weights) for different hiding field types.
Comment 9 Commit Notification 2018-08-31 11:06:24 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#118845: make HiddenPara have higher priority deciding visibility

It will be available in 6.2.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 10 Mike Kaganski 2018-08-31 11:07:53 UTC
https://gerrit.libreoffice.org/59865 is backport to 6-1.
Comment 11 Commit Notification 2018-09-01 23:04:44 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=481730bc4c9d63e831c088d9862b0f50f2da209f&h=libreoffice-6-1

tdf#118845: make HiddenPara have higher priority deciding visibility

It will be available in 6.1.2.

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.