Steps to reproduce: 1. Open attachment 129323 [details] from bug 90317 -> Table in first page has some space between the text and the top border. There shouldn't be any space. See comparison image Reproduced in Version: 6.5.0.0.alpha0+ Build ID: dee81fb2e1df5091702b3c8b0e4a3f2b58e89291 CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US Calc: threaded [Bug found by office-interoperability-tools]
Created attachment 156746 [details] comparison MSO 2010 and LibreOffice 6.5 Master
Regression introduced by: https://cgit.freedesktop.org/libreoffice/core/commit/?id=6bced3c6a1bf8d4652dd6ba75e41b128ce1bfc5c author Justin Luth <justin.luth@collabora.com> 2019-10-11 11:06:02 +0300 committer Miklos Vajna <vmiklos@collabora.com> 2019-11-27 11:57:10 +0100 commit 6bced3c6a1bf8d4652dd6ba75e41b128ce1bfc5c (patch) tree 2fdb545c37f5e0daf7f94bd65ff6101b824385eb parent 8408aea46e36c3fad689ffe47668422f65025795 (diff) tdf#118947 sw tablestyle: manually scan parents for ::SET Bisected with: bibisect-linux64-6.5 Adding Cc: to Justin Luth
Yes, this document shows the impossible situation we are in. Adding Laszlo since he has been working on these things in an alternate way. So the TableGrid style.xml here defines w:spacing=before=120, and direct formatting in document.xml resets it to the default value of w:spacing=0. Spacing is not defined anywhere else - not in docDefaults, or in the Default Format paragraph style. So the direct formatting matches LO default values, and since there is a single property that defines both before and after spacing we have no way of knowing whether the w:spacing=0 direct formatting is intentional or not. In order to fix this, we would need to explicitly track within writerfilter every shared-property that has been set - and also handle the table-grid overlay within writerfilter itself (instead of in SW) so that it can access the property-tracking variables.
Created attachment 156805 [details] ANSTO_reduced_directAfter.docx: modified so document.xml sets w:spacing after=0 instead of before=0 In this version, there SHOULD be space about the word "table". My patch (and before that Laszlo's patch for bug 119054) fixes this tweaked version of the proof-document. In this case the table-style refers to spacing BEFORE and the direct formatting refers to spacing AFTER. Since these two are tied together, LO was interpreting both BEFORE and AFTER as ::SET and not just a default value, so it overrode the tablestyle.
Created attachment 156806 [details] ANSTO_reduced_styleAfter.docx: modified so style.xml sets w:spacing after=0 instead of before=0 In this version, there SHOULD be space after the word "table". My patch (and before that Laszlo's patch for bug 119054) fixes this tweaked version of the proof-document. In this case the table-style refers to spacing AFTER and the direct formatting refers to spacing BEFORE. Since these two are tied together, LO was interpreting both BEFORE and AFTER as ::SET and not just a default value, so it overrode the tablestyle when it shouldn't have.
(In reply to Justin L from comment #3) > In order to fix this, we would need to explicitly track within writerfilter > every shared-property that has been set - and also handle the table-grid > overlay within writerfilter itself (instead of in SW) so that it can access > the property-tracking variables. That is basically what Laszlo had been doing, which isn't as hard as I thought - since this is just looking at PropertyNames and doesn't care about the implementation yet. So his approach looks very promising, but it needs testing against special table-style things like first-row/header etc... P.S. Laszlo's patch didn't actually fix the directAfter case (yet). Somehow I messed up my testing. Laszlo only looked at BOTTOM_MARGIN, and not TOP_MARGIN. Laszlo, do you want to take over, reverting my patch and extending your framework to cover pretty much any property and identifying when styles override their parents?
László Németh committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/f15d67442972c5f69c71925a6bfa5aa1a39d54eb tdf#129575 DOCX import: fix table style preference It will be available in 7.0.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.
@Justin, Xisco: I've started to extend my writerfilter-only patch, reverting the Justin's DEFAULT-based sw/UNO patch, which caused the regression. The submitted initial re-work fixes the regression and handles the attached test files well. I added the documents to the patch, as unit test documents. Also I added an extra docDefault/paragraph style unit test, too. There are still some problem here, for example, background color of tdf118947_tableStyle.docx, docDefault recognition doesn't work in table sub-styles (CNF masked properties), nested tables, so I'll continue the work. Thanks for your help.
Everything ok. Verified on Version: 6.4.3.2 Build ID: libreoffice-6.4.3.2-snap1 CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3; Locale: ro-RO (ro_RO.UTF-8); UI-Language: en-US Calc: threaded Also on Version: 7.0.0.0.alpha1 Build ID: 6a03b2a54143a9bc0c6d4c7f1... CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3; Locale: ro-RO (ro_RO.UTF-8); UI: en-US Calc: threaded