Bug 104199 - FILEOPEN: PPTX Table cell borders shouldn't be displayed
Summary: FILEOPEN: PPTX Table cell borders shouldn't be displayed
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.0.0.0.alpha0+ Master
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:6.2.0 target:6.1.0.1 target:6.0.6
Keywords: bibisected, bisected, filter:pptx, regression
: 108840 116128 (view as bug list)
Depends on:
Blocks: PPTX-Limitations
  Show dependency treegraph
 
Reported: 2016-11-27 13:41 UTC by Xisco Faulí
Modified: 2018-07-05 15:58 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
borders.pptx: colored slide-background with various table/cell fills created in PP2013 (33.84 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2018-06-02 16:16 UTC, Justin L
Details
exaggeratedBorders.pptx: nofill size 9pt borders (30.85 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2018-06-11 18:41 UTC, Justin L
Details
exaggeratedBordersMultirow.pptx: by hand edit of Office 2013 authored example (32.11 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2018-06-13 08:50 UTC, Justin L
Details
bnc480256.pptx: existing unit test which displayed the black border line problem in the last row. (35.15 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2018-06-14 07:13 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2016-11-27 13:41:44 UTC
Steps:
1. Open attachment 111945 [details]

Observed behaviour: Table cell borders are displayed

Reproduced in 

Version: 5.3.0.0.alpha1+
Build ID: 757a60d01dd152aadab2ba3c8224252481ce8a88
CPU Threads: 4; OS Version: Linux 4.8; UI Render: default; VCL: gtk3; Layout
Engine: new; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

but not in

Version: 4.3.0.0.alpha1+
Build ID: c15927f20d4727c3b8de68497b6949e72f9e6e9e
Comment 1 Xisco Faulí 2016-11-27 13:44:06 UTC
Regression introduced by:

author	yogesh.bharate001 <yogesh.bharate@synerzip.com>	2015-04-16 13:54:37 (GMT)
committer	Caolán McNamara <caolanm@redhat.com>	2015-05-09 19:55:31 (GMT)
commit	3e4d2043e99201ec542186039e3be34d3c226111 (patch)
tree	f0d9070c2fd0ae282ad118a05d2c7be737ae79d4
parent	3acb1d4b28944de908ffb3d0b756725ae015214f (diff)
tdf#90190 PPTX table cell border width is not exported.
Problem:
- Table cell border width is not exported.
i.e lnL, lnR, lnT, LnB are not exported inside the tcPr.

XML Difference:
Original :
<a:lnT w = "76200">

After RT : tag is missing.

Solution : Added solution for Table cell border width.

Adding CC: to yogesh.bharate001
Comment 2 Xisco Faulí 2017-06-28 20:25:17 UTC
*** Bug 108840 has been marked as a duplicate of this bug. ***
Comment 3 Xisco Faulí 2018-03-01 18:24:13 UTC
*** Bug 116128 has been marked as a duplicate of this bug. ***
Comment 4 Justin L 2018-06-02 12:51:46 UTC
proposed fix: https://gerrit.libreoffice.org/55205.  Both docs from the duplicate bugs look much better.
Comment 5 Justin L 2018-06-02 16:16:10 UTC
Created attachment 142488 [details]
borders.pptx: colored slide-background with various table/cell fills created in PP2013

PowerPoint gives you very LITTLE control over the table border properties, so it is hard to design an effective test document. I tried using the aBgColor value from pushToXCell() but I didn't see any visual improvements, so I left it at just blindly assigning COL_AUTO as a simple fix.
Comment 6 Justin L 2018-06-11 18:41:33 UTC
Created attachment 142661 [details]
exaggeratedBorders.pptx: nofill size 9pt borders

9pt borders are the maximum width possible.

The border size doesn't seem to affect the size of the cell, so I'm not sure what the value is of importing the invisible borders. It seems like it would be better not to import the border at all.  Removing that does not break any unit tests...
Comment 7 Justin L 2018-06-13 08:50:55 UTC
Created attachment 142695 [details]
exaggeratedBordersMultirow.pptx: by hand edit of Office 2013 authored example

Using COL_AUTO seems to work well with in-between-cell borders. It does not use the slide background color, but seems to use the cell background color. Perfect.
Comment 8 Commit Notification 2018-06-14 06:31:45 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#104199 sd: nofill borders shouldn't be visible.

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 9 Justin L 2018-06-14 07:13:09 UTC
Created attachment 142717 [details]
bnc480256.pptx: existing unit test which displayed the black border line problem in the last row.

There are two existing unit tests which use this code section. tableBorderLineStyle.pptx only uses this on TLBR / BLTR diagonal lines which LO can't show anyway, leaving only one usable QA example document to add to the example documents for this bug report.

Only Impress/Draw seems to use this code. Probably both Word and Excel use more elaborate table structures and don't use drawingML at all for their tables.

I'm debating whether I should attempt to backport this to stable 6.0.6, and I am leaning towards "yes" since every example I find validates the patch.
Comment 10 Commit Notification 2018-06-15 15:40:44 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

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

tdf#104199 sd: nofill borders shouldn't be visible.

It will be available in 6.1.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 11 Commit Notification 2018-06-22 17:05:10 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=05021abd97316fe3d188cd9f15c44db2170927d7&h=libreoffice-6-0

tdf#104199 sd: nofill borders shouldn't be visible.

It will be available in 6.0.6.

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 12 Justin L 2018-06-22 17:12:28 UTC
One more patch allows this to round-trip.  https://gerrit.libreoffice.org/55658
Comment 13 Commit Notification 2018-06-29 08:30:08 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#104199 sd: export non-borders as noFill

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 14 Justin L 2018-06-30 09:38:35 UTC
(In reply to Commit Notification from comment #13)
Note that this fix depends on this cleanup patch:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=3ef18b28ade43a38bb46a2400e4e81a9ae8796bc

Backports to 6.1 and 6.0 are awaiting review.
Comment 15 Commit Notification 2018-07-02 04:29:13 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

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

tdf#104199 sd: export non-borders as noFill

It will be available in 6.1.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 16 Xisco Faulí 2018-07-02 23:25:45 UTC
Verified in

Version: 6.2.0.0.alpha0+
Build ID: 5fce97a58b8f764e35bf98128591c9a89537da05
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded

@Justin, thanks for fixing this!!
Comment 17 Commit Notification 2018-07-05 15:58:03 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fb043986ca8d2d1b7b6179ef7e2412aa677d5c7d&h=libreoffice-6-0

tdf#104199 sd: export non-borders as noFill

It will be available in 6.0.6.

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.