diff --git a/CHANGELOG.md b/CHANGELOG.md index d0850fb5..44cc3c70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- [SIL.LCModel] BaseStyleInfo now loads default font features (ktptFontVariations) from style rules, fixing font features set as style defaults being dropped on reload (LT-22351) +- [SIL.LCModel] BaseStyleInfo no longer clobbers ws-specific font overrides inherited from a based-on style with the default font info's merely-inherited values; only explicitly-set default properties are propagated to ws overrides (LT-22351) +- [SIL.LCModel] BulletInfo now decodes all properties of an encoded bullet font info instead of stopping at the first string property, fixing bullet font features (ktptFontVariations) being lost when a bullet font name was also set (LT-22351) - [SIL.LCModel] Deleting a duplicated phonological rule no longer deletes pooled feature constraints and environment contexts the original rule still references (LT-22575) - [SIL.LCModel] Fixed crash when bulk deleting entries involved in a lexical relation (LT-21598) - [SIL.LCModel.FixData] Find and Fix removes duplicate Targets from a LexReference (and deletes it if fewer than two distinct Targets remain) (LT-21598) diff --git a/src/SIL.LCModel/DomainServices/BaseStyleInfo.cs b/src/SIL.LCModel/DomainServices/BaseStyleInfo.cs index 8bb24098..abd3633e 100644 --- a/src/SIL.LCModel/DomainServices/BaseStyleInfo.cs +++ b/src/SIL.LCModel/DomainServices/BaseStyleInfo.cs @@ -935,6 +935,10 @@ private void ProcessStyleRules(ITsTextProps styleProps, m_defaultFontInfo.m_fontName.ExplicitValue = sProp; break; + case (int)FwTextPropType.ktptFontVariations: + SetFontStringProp(tpt, m_defaultFontInfo, sProp); + break; + case (int)FwTextPropType.ktptBulNumTxtBef: { m_bulletInfo.IsInherited = false; @@ -1436,7 +1440,11 @@ private void SetNonExplicitPropertiesToInherited() fontInfoOverride.Value.InheritAllProperties(inheritPropsFrom); if (m_defaultFontInfo.IsAnyExplicit) { - fontInfoOverride.Value.InheritAllProperties(m_defaultFontInfo); + // Propagate only the explicitly-set default (style-level) properties into the + // override; the default's other values were merely inherited from the based-on + // style's default and must not clobber what was just inherited from the + // based-on style's ws override. + fontInfoOverride.Value.InheritExplicitProperties(m_defaultFontInfo); } } m_rtl.InheritValue(basedOn.m_rtl); diff --git a/src/SIL.LCModel/DomainServices/BulletInfo.cs b/src/SIL.LCModel/DomainServices/BulletInfo.cs index f882f1f8..7fc04149 100644 --- a/src/SIL.LCModel/DomainServices/BulletInfo.cs +++ b/src/SIL.LCModel/DomainServices/BulletInfo.cs @@ -210,13 +210,13 @@ private static FontInfo DecodeFontInfo(string blob) { fontInfo.m_fontName.ExplicitValue = blob.Substring(i, iPropLim - i); i += (iPropLim - i + 1); - break; + continue; } else if (tpt == FwTextPropType.ktptFontVariations) { fontInfo.m_features.ExplicitValue = blob.Substring(i, iPropLim - i); i += (iPropLim - i + 1); - break; + continue; } int nVal = (int)blob[i] + (int)(blob[i + 1] << 16); diff --git a/src/SIL.LCModel/DomainServices/FontInfo.cs b/src/SIL.LCModel/DomainServices/FontInfo.cs index 474d1986..b8f49bfa 100644 --- a/src/SIL.LCModel/DomainServices/FontInfo.cs +++ b/src/SIL.LCModel/DomainServices/FontInfo.cs @@ -273,5 +273,40 @@ internal void InheritAllProperties(FontInfo basedOnFontInfo) m_offset.InheritValue(basedOnFontInfo.m_offset); m_features.InheritValue(basedOnFontInfo.m_features); } + + /// ------------------------------------------------------------------------------------ + /// + /// Inherit the values of only those properties that are explicitly set on the specified + /// font info, but don't force a change in the inherit value. + /// + /// Explicitly set values on this font info will not be affected + /// The font info from which to get the explicitly set + /// values. + /// ------------------------------------------------------------------------------------ + internal void InheritExplicitProperties(FontInfo basedOnFontInfo) + { + if (basedOnFontInfo.m_fontName.IsExplicit) + m_fontName.InheritValue(basedOnFontInfo.m_fontName); + if (basedOnFontInfo.m_fontSize.IsExplicit) + m_fontSize.InheritValue(basedOnFontInfo.m_fontSize); + if (basedOnFontInfo.m_fontColor.IsExplicit) + m_fontColor.InheritValue(basedOnFontInfo.m_fontColor); + if (basedOnFontInfo.m_backColor.IsExplicit) + m_backColor.InheritValue(basedOnFontInfo.m_backColor); + if (basedOnFontInfo.m_bold.IsExplicit) + m_bold.InheritValue(basedOnFontInfo.m_bold); + if (basedOnFontInfo.m_italic.IsExplicit) + m_italic.InheritValue(basedOnFontInfo.m_italic); + if (basedOnFontInfo.m_superSub.IsExplicit) + m_superSub.InheritValue(basedOnFontInfo.m_superSub); + if (basedOnFontInfo.m_underline.IsExplicit) + m_underline.InheritValue(basedOnFontInfo.m_underline); + if (basedOnFontInfo.m_underlineColor.IsExplicit) + m_underlineColor.InheritValue(basedOnFontInfo.m_underlineColor); + if (basedOnFontInfo.m_offset.IsExplicit) + m_offset.InheritValue(basedOnFontInfo.m_offset); + if (basedOnFontInfo.m_features.IsExplicit) + m_features.InheritValue(basedOnFontInfo.m_features); + } } } diff --git a/tests/SIL.LCModel.Tests/DomainServices/BaseStyleInfoTests.cs b/tests/SIL.LCModel.Tests/DomainServices/BaseStyleInfoTests.cs index 87ba5b6a..b7de9e3e 100644 --- a/tests/SIL.LCModel.Tests/DomainServices/BaseStyleInfoTests.cs +++ b/tests/SIL.LCModel.Tests/DomainServices/BaseStyleInfoTests.cs @@ -274,6 +274,35 @@ public void CreateCopy() Assert.AreEqual(false, newInfo.IsBuiltIn, "Copies of styles should not be considered built in"); } + /// ------------------------------------------------------------------------------------ + /// + /// Tests that font features (ktptFontVariations) set as a style-level default (not a + /// writing-system override) are loaded into the default font info. See LT-22351: such + /// features were saved to the database but silently dropped when the style was reloaded, + /// because ProcessStyleRules had no case for ktptFontVariations. + /// + /// ------------------------------------------------------------------------------------ + [Test] + public void ConstructBasedOnStyle_FontFeatures() + { + ITsPropsBldr props; + + IStStyle mainTitleStyle = AddTestStyle("Title Main", ContextValues.Title, + StructureValues.Body, FunctionValues.Prose, false, Cache.LangProject.StylesOC); + props = mainTitleStyle.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptFontFamily, "Arial"); + props.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "smcp=1,ss01=2"); + mainTitleStyle.Rules = props.GetTextProps(); + + DummyStyleInfo entry = new DummyStyleInfo(mainTitleStyle); + FontInfo fontInfo = entry.FontInfoForWs(-1); + Assert.IsNotNull(fontInfo); + Assert.IsTrue(fontInfo.m_features.IsExplicit, + "Default font features set on the style should be explicit, not inherited"); + Assert.AreEqual("smcp=1,ss01=2", fontInfo.m_features.Value); + Assert.AreEqual("Arial", (string)fontInfo.m_fontName.Value); + } + /// /// Minimal test of an alternate constructor to verify that it records the cache from the style. /// @@ -402,6 +431,178 @@ public void SetBasedOnStyleAndInheritValues_WsOverrideInChildInheritsParentValue "The child entry in the StyleInfoCollection did not inherit the fontName properly"); } + /// + /// Tests that a child style whose only explicit style-level default is font features + /// (ktptFontVariations) does not clobber the ws-specific overrides it inherits from its + /// based-on style. The explicitly-set default properties should be propagated to the ws + /// overrides, but the default's merely-inherited values must not overwrite what was just + /// inherited from the based-on style's ws override. See LT-22351. + /// + [Test] + public void SetBasedOnStyleAndInheritValues_ExplicitDefaultFeaturesDoNotClobberWsOverride() + { + // Font family override for the parent style + byte[] buffer = new byte[] { + 0xC1, 0x87, 0x8B, 0x3B, // WS for english (little endian) + 0x09, 0x00, // FF length + (byte)'F', 0, + (byte)'o', 0, + (byte)'n', 0, + (byte)'t', 0, + (byte)'a', 0, + (byte)'s', 0, + (byte)'t', 0, + (byte)'i', 0, + (byte)'c', 0, + + 0x00, 0x00, // Count of int props + }; + + ITsPropsBldr props; + + var mainTitleStyle = AddTestStyle("Title Main", ContextValues.Title, + StructureValues.Body, FunctionValues.Prose, false, Cache.LangProject.StylesOC); + var inheritFromMain = AddTestStyle("Inherit Title", ContextValues.Title, StructureValues.Body, + FunctionValues.Prose, false, Cache.LangProject.StylesOC); + inheritFromMain.BasedOnRA = mainTitleStyle; + // The parent style has a ws-specific font override and a different default font + props = mainTitleStyle.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptFontFamily, "Times New Roman"); + props.SetStrPropValue((int)FwTextPropType.ktptWsStyle, DummyStyleInfo.MakeStringFromBuffer(buffer)); + mainTitleStyle.Rules = props.GetTextProps(); + // The child style's only explicit style-level default is font features + props = inheritFromMain.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "smcp=1"); + inheritFromMain.Rules = props.GetTextProps(); + + var entry = new DummyStyleInfo(mainTitleStyle); + var childEntry = new DummyStyleInfo(inheritFromMain); + var styleInfos = new LcmStyleSheet.StyleInfoCollection(); + styleInfos.Add(entry); + styleInfos.Add(childEntry); + // SUT + childEntry.SetBasedOnStyleAndInheritValues(styleInfos); + var childOverrides = childEntry.FontInfoForWs(Cache.DefaultAnalWs); + Assert.True(childOverrides.m_fontName.IsInherited, + "The child entry's ws override should still inherit the fontName"); + Assert.AreEqual("Fontastic", childOverrides.m_fontName.Value, + "The fontName inherited from the parent's ws override should not be clobbered by the child's default font info"); + Assert.AreEqual("smcp=1", childOverrides.m_features.Value, + "The child's explicit default font features should be propagated to its ws override"); + } + + /// + /// Tests that a child style's explicit style-level default properties are still + /// propagated into its ws-specific overrides (the intended behavior of LT-18109), + /// while properties the child does not set explicitly keep the values inherited from + /// the based-on style's ws override. See LT-22351. + /// + [Test] + public void SetBasedOnStyleAndInheritValues_ExplicitDefaultPropsPropagateIntoWsOverride() + { + // Font family override for the parent style + byte[] buffer = new byte[] { + 0xC1, 0x87, 0x8B, 0x3B, // WS for english (little endian) + 0x09, 0x00, // FF length + (byte)'F', 0, + (byte)'o', 0, + (byte)'n', 0, + (byte)'t', 0, + (byte)'a', 0, + (byte)'s', 0, + (byte)'t', 0, + (byte)'i', 0, + (byte)'c', 0, + + 0x00, 0x00, // Count of int props + }; + + ITsPropsBldr props; + + var mainTitleStyle = AddTestStyle("Title Main", ContextValues.Title, + StructureValues.Body, FunctionValues.Prose, false, Cache.LangProject.StylesOC); + var inheritFromMain = AddTestStyle("Inherit Title", ContextValues.Title, StructureValues.Body, + FunctionValues.Prose, false, Cache.LangProject.StylesOC); + inheritFromMain.BasedOnRA = mainTitleStyle; + // The parent style has a ws-specific font override and a different default font + props = mainTitleStyle.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptFontFamily, "Times New Roman"); + props.SetStrPropValue((int)FwTextPropType.ktptWsStyle, DummyStyleInfo.MakeStringFromBuffer(buffer)); + mainTitleStyle.Rules = props.GetTextProps(); + // The child style's only explicit style-level default is bold, which the parent's + // ws override does not set + props = inheritFromMain.Rules.GetBldr(); + props.SetIntPropValues((int)FwTextPropType.ktptBold, + (int)FwTextPropVar.ktpvDefault, 1); + inheritFromMain.Rules = props.GetTextProps(); + + var entry = new DummyStyleInfo(mainTitleStyle); + var childEntry = new DummyStyleInfo(inheritFromMain); + var styleInfos = new LcmStyleSheet.StyleInfoCollection(); + styleInfos.Add(entry); + styleInfos.Add(childEntry); + // SUT + childEntry.SetBasedOnStyleAndInheritValues(styleInfos); + var childOverrides = childEntry.FontInfoForWs(Cache.DefaultAnalWs); + Assert.AreEqual("Fontastic", childOverrides.m_fontName.Value, + "The fontName inherited from the parent's ws override should not be clobbered by the child's default font info"); + Assert.IsTrue(childOverrides.m_bold.Value, + "The child's explicit default bold setting should be propagated to its ws override"); + } + + /// + /// Tests that a child style's explicit style-level default font name applies across + /// writing systems: it wins over the font name in the based-on style's ws override. + /// See LT-22351. + /// + [Test] + public void SetBasedOnStyleAndInheritValues_ExplicitDefaultFontNameBeatsParentWsOverride() + { + // Font family override for the parent style + byte[] buffer = new byte[] { + 0xC1, 0x87, 0x8B, 0x3B, // WS for english (little endian) + 0x09, 0x00, // FF length + (byte)'F', 0, + (byte)'o', 0, + (byte)'n', 0, + (byte)'t', 0, + (byte)'a', 0, + (byte)'s', 0, + (byte)'t', 0, + (byte)'i', 0, + (byte)'c', 0, + + 0x00, 0x00, // Count of int props + }; + + ITsPropsBldr props; + + var mainTitleStyle = AddTestStyle("Title Main", ContextValues.Title, + StructureValues.Body, FunctionValues.Prose, false, Cache.LangProject.StylesOC); + var inheritFromMain = AddTestStyle("Inherit Title", ContextValues.Title, StructureValues.Body, + FunctionValues.Prose, false, Cache.LangProject.StylesOC); + inheritFromMain.BasedOnRA = mainTitleStyle; + // The parent style has a ws-specific font override + props = mainTitleStyle.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptWsStyle, DummyStyleInfo.MakeStringFromBuffer(buffer)); + mainTitleStyle.Rules = props.GetTextProps(); + // The child style has an explicit style-level default font name + props = inheritFromMain.Rules.GetBldr(); + props.SetStrPropValue((int)FwTextPropType.ktptFontFamily, "Times New Roman"); + inheritFromMain.Rules = props.GetTextProps(); + + var entry = new DummyStyleInfo(mainTitleStyle); + var childEntry = new DummyStyleInfo(inheritFromMain); + var styleInfos = new LcmStyleSheet.StyleInfoCollection(); + styleInfos.Add(entry); + styleInfos.Add(childEntry); + // SUT + childEntry.SetBasedOnStyleAndInheritValues(styleInfos); + var childOverrides = childEntry.FontInfoForWs(Cache.DefaultAnalWs); + Assert.AreEqual("Times New Roman", childOverrides.m_fontName.Value, + "The child's explicit default font name should apply across writing systems, winning over the parent's ws override"); + } + /// ------------------------------------------------------------------------------------ /// /// Tests retrieving WS specific overrides from string diff --git a/tests/SIL.LCModel.Tests/DomainServices/BulletInfoTests.cs b/tests/SIL.LCModel.Tests/DomainServices/BulletInfoTests.cs index 71d9a90d..fce53efa 100644 --- a/tests/SIL.LCModel.Tests/DomainServices/BulletInfoTests.cs +++ b/tests/SIL.LCModel.Tests/DomainServices/BulletInfoTests.cs @@ -139,6 +139,37 @@ public void RoundTripEncodingAndDecodingOfFontInfo() Assert.AreEqual(expectedFontInfo, bulletInfo2.FontInfo); } + + /// ------------------------------------------------------------------------------------ + /// + /// Tests that we can save and restore a font info that has both a font name and font + /// features (ktptFontVariations) explicitly set. DecodeFontInfo used to stop parsing + /// after the first string property, so the font features (which are encoded after the + /// font name) were lost on every decode. See LT-22351. + /// + /// ------------------------------------------------------------------------------------ + [Test] + public void RoundTripEncodingAndDecodingOfFontInfo_FontNameAndFontFeatures() + { + // Setup what we expect + FontInfo expectedFontInfo = m_infoTable["TestStyle"].FontInfoForWs(-1); + expectedFontInfo.m_fontName = new InheritableStyleProp("Algerian"); + expectedFontInfo.m_features = new InheritableStyleProp("smcp=1,ss01=2"); + + BulletInfo bulletInfo1 = new BulletInfo(); + bulletInfo1.FontInfo = expectedFontInfo; + + BulletInfo bulletInfo2 = new BulletInfo(); + bulletInfo2.EncodedFontInfo = bulletInfo1.EncodedFontInfo; + + Assert.IsTrue(bulletInfo2.FontInfo.m_fontName.IsExplicit, + "The font name should survive the round trip"); + Assert.AreEqual("Algerian", bulletInfo2.FontInfo.m_fontName.Value); + Assert.IsTrue(bulletInfo2.FontInfo.m_features.IsExplicit, + "The font features should survive the round trip"); + Assert.AreEqual("smcp=1,ss01=2", bulletInfo2.FontInfo.m_features.Value); + Assert.AreEqual(expectedFontInfo, bulletInfo2.FontInfo); + } #endregion } }