Bug 109184 - FILEOPEN: DOCX - Table cells imported with white background when it should be no fill
Summary: FILEOPEN: DOCX - Table cells imported with white background when it should be...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.2.8.2 release
Hardware: All All
: medium normal
Assignee: Aron Budea
URL:
Whiteboard: target:6.0.0 target:5.4.2
Keywords: filter:docx
Depends on:
Blocks: OOXML-Object-Fill DOCX-Tables
  Show dependency treegraph
 
Reported: 2017-07-18 08:21 UTC by Buovjaga
Modified: 2017-09-27 21:41 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
DOCX bug document (45.69 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-07-18 08:21 UTC, Buovjaga
Details
Sample DOCX (18.79 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-08-16 16:19 UTC, Aron Budea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Buovjaga 2017-07-18 08:21:51 UTC
Created attachment 134705 [details]
DOCX bug document

1. Open attached file
2. Save as DOCX and reload

Table got white background and Helsinki logo is obscured.

Testing with old versions:
- 3.5 does not have the table in the header.
- 4.1 crashes upon save

Version: 4.4.5.2
Build ID: a22f674fd25a3b6f45bdebf25400ed2adff0ff99
Locale: fi_FI

Win 7 Pro 64-bit Version: 6.0.0.0.alpha0+ (x64)
Build ID: 7a743b472dadb817eb7a6ed8063cee80ce7412e8
CPU threads: 4; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2017-07-18_01:01:15
Locale: fi-FI (fi_FI); Calc: CL
Comment 1 Xisco Faulí 2017-07-18 09:32:35 UTC
Confirmed in

Version: 6.0.0.0.alpha0+
Build ID: ddadcb4f4a2bc6538c219a0a577bdf5999015150
CPU threads: 4; OS: Linux 4.8; UI render: default; VCL: gtk2; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 2 Jacques Guilleron 2017-07-18 09:35:55 UTC
I reproduce with
LO 6.0.0.0.alpha0+ Build ID: 643da8ec4e721d33dfdf8d78bedd50a915f1188d
CPU threads: 2; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2017-06-26_01:24:22
Locale: fr-FR (fr_FR); Calc: CL
and
LO 4.2.8.1 Build ID: 4044db1653798618515c987464157abee9229c11
Changing Bacckground to No fill into Table Properties doen't modify this behaviour.
Comment 3 Yousuf Philips (jay) 2017-07-18 15:12:54 UTC
This issue happens with all tables in the document and not just the ones in the header, though its only noticeable by default in the header as there is an image behind the table. Also this is a file import problem as opening the LO saved DOCX file in Word 2010 looks fine.

The problem boils down to LO not being able to handle the set values of the fill and val attributes of the <w:shd> tag, as shown in the xml snippet below.

<w:tbl>
 <w:tr>
  <w:tc>
   <w:tcPr>
    <...>
    <w:shd w:fill="auto" w:val="clear" />
    <...>
   </w:tcPr>
  </w:tc>
 </w:tr>
</w:tbl>

Justin, Mike, Miklos: Any of you want to take this issue on?
Comment 4 Aron Budea 2017-07-24 04:27:07 UTC
(In reply to Yousuf Philips (jay) from comment #3)
> an image behind the table. Also this is a file import problem as opening the
> LO saved DOCX file in Word 2010 looks fine.

It would also make sense to not export such a w:shd attribute combination, at least when it's set on table level.
Comment 5 Aron Budea 2017-08-16 16:19:33 UTC
Created attachment 135592 [details]
Sample DOCX

Attaching minimal repro case. Save and reopen, and the table background becomes white.

The assumption is certainly incorrect here:
https://opengrok.libreoffice.org/xref/core/writerfilter/source/dmapper/CellColorHandler.cxx#113
nIntValue = 0xffffff; //fill color auto means white

However, my thought was to handle it in the following part by not adding it to the property map, similarly as a few lines before inside the if (eg. the color set beforehand doesn't matter):
https://opengrok.libreoffice.org/xref/core/writerfilter/source/dmapper/CellColorHandler.cxx#282
pPropertyMap->Insert( m_OutputFormat == Form ? PROP_BACK_COLOR
                    : PROP_CHAR_BACK_COLOR, uno::makeAny( nApplyColor ));
Comment 6 Buovjaga 2017-08-16 18:42:24 UTC
(In reply to Aron Budea from comment #5)
> The assumption is certainly incorrect here:
> https://opengrok.libreoffice.org/xref/core/writerfilter/source/dmapper/
> CellColorHandler.cxx#113
> nIntValue = 0xffffff; //fill color auto means white

Related: bug 106079
https://cgit.freedesktop.org/libreoffice/core/commit/?id=2f200f1e760477300ea0a6eb75cf0bf6841cf81f
Comment 7 Commit Notification 2017-08-23 10:17:18 UTC
Szymon Kłos committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109184 auto cell color should be transparent

It will be available in 6.0.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 8 Yousuf Philips (jay) 2017-08-24 14:04:06 UTC
(In reply to Aron Budea from comment #4)
> It would also make sense to not export such a w:shd attribute combination,
> at least when it's set on table level.

Yes it shouldnt export this if it isnt explicitly set, so please file a new bug report for this export issue.
Comment 9 Jacques Guilleron 2017-08-24 14:41:35 UTC
Works fine now with
LO 6.0.0.0.alpha0+ Build ID: 02c53f744ed23e2149fc7c83d67cb7d8aa5eb0ed
CPU threads: 2; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2017-08-24_05:48:55
Locale: fr-FR (fr_FR); Calc: CL
Thanks to Szymon
Comment 10 Commit Notification 2017-09-13 19:47:49 UTC
Szymon Kłos committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

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

tdf#109184 auto cell color should be transparent

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