Bug 128153 - FILEOPEN: DOCX: Undefined style wrongly inherits from default paragraph style
Summary: FILEOPEN: DOCX: Undefined style wrongly inherits from default paragraph style
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:6.5.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks: 118947
  Show dependency treegraph
 
Reported: 2019-10-15 11:23 UTC by Justin L
Modified: 2020-09-02 20:23 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
noFontDefaults.docx: hand-modified to test the accuracy of some suspicious commits (7.33 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2019-10-15 11:23 UTC, Justin L
Details
noFontDefaults.png: screenshot of Word 2016 showing TimesNewRoman 10pt. (31.48 KB, image/png)
2019-10-15 11:27 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2019-10-15 11:23:06 UTC
Created attachment 155019 [details]
noFontDefaults.docx: hand-modified to test the accuracy of some suspicious commits

In Word 2003 and 2016, the paragraph using a custom style with no parent uses program-defaults of Times New Roman 10pt font. In LO, this is currently using the Default-paragraph-style size of 32pt, and a hard-coded Calibri font.

The fontsize was broken by LO 4.3 commit 8766011bccfd0f12f8dd77d2f94eb16e2e8c3471
Author:     Miklos Vajna
CommitDate: Wed Jan 22 16:21:51 2014 +0100 
  DOCX import: set document-level font size default based on default para style

This document contains
1.) no DocDefault information about sz (which is normal)
2.) the "Normal" paragraph style is marked as w:default=1 (which is normal) and has a w:sz defined.
3.) a "NoParent" paragraph style that has no w:sz defined.

There are a couple of things wrong with the regressive patch.
1.) although this matches what binary import format does, the entire premise seems to be false, as this document proves.
2.) m_pDefaultParaProps should never have a CHAR_HEIGHT property. That would be set in m_pDefaultCharProps.


The fontsize changed to Liberation Sans in LO 4.3 which based on the commit descriptions sounds like it is a valid change since the font metrics should be identical to TNR. 
Commit 06299d6c1620e5b2f2a3588d7c93790278397cbd
Author:     Samuel Mehrbrodt
CommitDate: Wed Dec 4 05:06:50 2013 -0600
  Prefer the Liberation fonts over Arial/Times
        The new Writer template was supposed to use the Liberation fonts: http://nabble.documentfoundation.org/Default-Writer-Template-td4076271.html
        So I changed the defaults to prefer them over their MS equivalents. When they are not available, they will be substituted as used.
    
        This should not cause problems since the Liberation Fonts have the same metrics as Times, Arial and Courier New.


It broke in the change from TNR/Liberation-Sans to Calibri in LO 5.4 commits
commit ec6ff5ec2aa182a8540ad11e61c9f982264f3da2
Author: Mike Kaganski
Date:   Tue Jun 6 16:19:31 2017 +0200
    tdf#104450: Use Calibri; let LO to fallback to Carlito
    
    Using Calibri will allow to keep originally intended font
    on round-trip. If Calibri is absent on a system, LO will
    fallback to Carlito for rendering, but keep original font
    intact.
 
commit 6f2ad89b33d972f9642bb53eeb91f41df3b6b0e6
Author: Mike Kaganski
Date:   Mon Jun 5 23:47:13 2017 +0300
    tdf#108350: Use Carlito for DOCX import by default
    
    In OOXML (i.e. Word since 2007), the default document font is Calibri
    11 pt. If a document doesn't contain font information, we should assume
    our metric-compatible equivalent Carlito to provide best layout match.
    
    A unit test included.
    
    An existing unit test (testN766487) was corrected to match the font
    size that Word uses (11; was 12 which doesn't match Word's size).
Comment 1 Justin L 2019-10-15 11:27:49 UTC
Created attachment 155020 [details]
noFontDefaults.png: screenshot of Word 2016 showing TimesNewRoman 10pt.
Comment 2 Justin L 2019-10-15 13:01:47 UTC
For default fontname, the key differentiator between Mike's example document was that it consisted of only a document.xml file. The absence of styles.xml triggers the fallback to Calibri/11. The presence of styles.xml should trigger Times New Roman/10.
Comment 3 Justin L 2019-10-15 18:14:48 UTC
(In reply to Justin L from comment #2)
> The absence of styles.xml triggers the fallback to Calibri/11.
More specifically, it is the DocDefaults-rPrDefaults section that is required.

The proposed patch for this references bug 108350 instead of this bug report. So this report can focus on the fontsize.
https://gerrit.libreoffice.org/80854
Comment 4 Xisco Faulí 2019-10-16 08:40:47 UTC
Reproduced in

Version: 6.4.0.0.alpha0+
Build ID: 4704acf63f4fed3a99fc95ff63c82eb5a9ae3908
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
Comment 5 Justin L 2019-10-23 09:08:06 UTC
I'll wait for LO 6.5 master before submitting this proposed fix at https://gerrit.libreoffice.org/81365
Comment 6 Commit Notification 2019-11-22 09:48:31 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/334409fbde555a957cd34e295cc27f2c2bf6e194

tdf#128153 docx/VML: apply style properties to shape text

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