include/editeng/escapementitem.hxx | 4 ++-- sw/qa/extras/htmlimport/data/tdf80194_subscript.html | 4 ++++ sw/qa/extras/htmlimport/htmlimport.cxx | 19 +++++++++++++++++++ sw/source/filter/ww8/docxattributeoutput.cxx | 2 +- sw/source/filter/ww8/ww8atr.cxx | 2 +- 5 files changed, 27 insertions(+), 4 deletions(-)
New commits: commit 5f4a65b7a3a29972c90a5ef4eb5fd7795b205cdf Author: Justin Luth <justin_l...@sil.org> AuthorDate: Mon Feb 17 12:52:28 2020 +0300 Commit: Justin Luth <justin_l...@sil.org> CommitDate: Tue Feb 18 19:31:45 2020 +0100 tdf#80194 UI: revert subscript DFLT_ESC_SUB to 8% (from 33%) Originally this was at 8%, but long ago for some reason that I couldn't find, it was changed to match superscript's 33%. This primarily affects UI for editeng. 33% was completely wrong. It puts the subscript WAY too low. The font's descent is only 20% of the total font height, so to lower the character by 33% pushes it below the line instead of towards the bottom of the line. Many export fixes that match the formula which calculated the values for DFLT_ESC_AUTO_* were made to LO6.4 for tdf#127316. Changing this default suprisingly has almost no effect. Import and export do not depend on a specific escapement to determine whether to treat it as automatic or not. That is generally handled with special keywords, or in the case of RTF with a +1 on the nPROP value. Writerfilter's use is in a never-to-be-encountered failsafe edge-case. HTML's import is affected, but for the good, so I made it into a unit test. Primarily it affects the UI dialog - suggesting 8% when automatic checkbox is cleared, and the biggest impact comes with the toolbar button for Draw/Impress which don't set "automatic" mode, so they just take the DFLT_ESC_SUB value. Looks much better now. And that was the heart of the bug report - that editeng defaults for subscript were way off. Change-Id: I6769072d483467e86fea82dfc534eb5e04802491 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88910 Tested-by: Jenkins Reviewed-by: Justin Luth <justin_l...@sil.org> diff --git a/include/editeng/escapementitem.hxx b/include/editeng/escapementitem.hxx index 6811511c4076..ea8f99e3519c 100644 --- a/include/editeng/escapementitem.hxx +++ b/include/editeng/escapementitem.hxx @@ -25,8 +25,8 @@ // class SvxEscapementItem ----------------------------------------------- -#define DFLT_ESC_SUPER 33 // 1/3 -#define DFLT_ESC_SUB -33 // also 1/3 previously 8/100 +#define DFLT_ESC_SUPER 33 // 42% (100 - DFLT_ESC_PROP) of ascent (~80% of font height) = 33% of total font height +#define DFLT_ESC_SUB -8 // 42% of descent (~20% of font height) = -8%. previously -33% (pre-2020), previously 8/100 (pre-2000?) #define DFLT_ESC_PROP 58 #define MAX_ESC_POS 13999 #define DFLT_ESC_AUTO_SUPER (MAX_ESC_POS+1) diff --git a/sw/qa/extras/htmlimport/data/tdf80194_subscript.html b/sw/qa/extras/htmlimport/data/tdf80194_subscript.html new file mode 100644 index 000000000000..5b00f5dd8d3f --- /dev/null +++ b/sw/qa/extras/htmlimport/data/tdf80194_subscript.html @@ -0,0 +1,4 @@ +<font size=+4> + <p>Does the subscript go lower than the descent? q<sub>p</sub>.</p> + <p>We want to know about the ascent for a superscripT<sup>L</sup> also.</p> +</font> diff --git a/sw/qa/extras/htmlimport/htmlimport.cxx b/sw/qa/extras/htmlimport/htmlimport.cxx index 86e123e62e59..6259b1141520 100644 --- a/sw/qa/extras/htmlimport/htmlimport.cxx +++ b/sw/qa/extras/htmlimport/htmlimport.cxx @@ -317,6 +317,25 @@ DECLARE_HTMLIMPORT_TEST(testReqIfBr, "reqif-br.xhtml") CPPUNIT_ASSERT(getParagraph(1)->getString().startsWith("aaa\nbbb")); } +DECLARE_HTMLIMPORT_TEST(testTdf80194_subscript, "tdf80194_subscript.html") +{ + uno::Reference<text::XTextRange> xPara = getParagraph(1); + CPPUNIT_ASSERT_DOUBLES_EQUAL( 0.f, getProperty<float>(getRun(xPara, 1), "CharEscapement"), 0); + // Most recently, the default subscript was 33%, which is much too large for a subscript. + // The original 8% (derived from a mathematical calculation) is much better in general, + // and for HTML was a better match when testing with firefox. + // DFLT_ESC_AUTO_SUB was tested, but HTML specs are pretty loose, and generally + // it exceeds the font ascent - so the formula-based-escapement is not appropriate. + CPPUNIT_ASSERT_DOUBLES_EQUAL( -8.f, getProperty<float>(getRun(xPara, 2, "p"), "CharEscapement"), 1); + + xPara.set(getParagraph(2)); + CPPUNIT_ASSERT_DOUBLES_EQUAL( 0.f, getProperty<float>(getRun(xPara, 1), "CharEscapement"), 0); + uno::Reference<text::XTextRange> xRun (getRun(xPara, 2, "L")); + CPPUNIT_ASSERT_DOUBLES_EQUAL( 33.f, getProperty<float>(xRun, "CharEscapement"), 1); + // HTML (although unspecified) tends to use a fairly large font. Definitely more than DFLT_ESC_PROP. + CPPUNIT_ASSERT( 70 < getProperty<sal_Int8>(xRun, "CharEscapementHeight")); +} + DECLARE_HTMLIMPORT_TEST(testReqIfTable, "reqif-table.xhtml") { // to see this: soffice --infilter="HTML (StarWriter):xhtmlns=reqif-xhtml" sw/qa/extras/htmlimport/data/reqif-table.xhtml diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx index 5fe8e2f41088..dc56f3664e66 100644 --- a/sw/source/filter/ww8/docxattributeoutput.cxx +++ b/sw/source/filter/ww8/docxattributeoutput.cxx @@ -6866,7 +6866,7 @@ void DocxAttributeOutput::CharEscapement( const SvxEscapementItem& rEscapement ) { // Lowered by the differences between the descenders (descent = baseline to bottom of lowest letter). // The descent is generally about 20% of the total font height. - // That is why DFLT_ESC_PROP (58) _originally_ lead to 8% (DFLT_ESC_SUB) + // That is why DFLT_ESC_PROP (58) leads to 8% (DFLT_ESC_SUB) nEsc = .2 * -(100 - nProp); } diff --git a/sw/source/filter/ww8/ww8atr.cxx b/sw/source/filter/ww8/ww8atr.cxx index 059bca60c954..f1d1222fcf64 100644 --- a/sw/source/filter/ww8/ww8atr.cxx +++ b/sw/source/filter/ww8/ww8atr.cxx @@ -1414,7 +1414,7 @@ void WW8AttributeOutput::CharEscapement( const SvxEscapementItem& rEscapement ) { // Lowered by the differences between the descenders (descent = baseline to bottom of lowest letter). // The descent is generally about 20% of the total font height. - // That is why DFLT_ESC_PROP (58) _originally_ lead to 8% (DFLT_ESC_SUB) + // That is why DFLT_ESC_PROP (58) leads to 8% (DFLT_ESC_SUB) nEsc = .2 * -(100 - nProp); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits