Diff
Modified: trunk/LayoutTests/ChangeLog (266682 => 266683)
--- trunk/LayoutTests/ChangeLog 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/LayoutTests/ChangeLog 2020-09-06 22:20:54 UTC (rev 266683)
@@ -1,3 +1,20 @@
+2020-09-06 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Letter-spacing should disable ligatures
+ https://bugs.webkit.org/show_bug.cgi?id=176215
+ <rdar://problem/17044265>
+
+ Reviewed by Antti Koivisto.
+
+ imported/w3c/web-platform-tests/css/css-fonts/font-feature-resolution-001.html passes now.
+
+ * TestExpectations:
+ * platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/css-style-attr-decl-block-expected.txt:
+ Apparently this test is sensitive to the ordering of CSSPropertyID enum values in CSSPropertyNames.h.
+ I filed https://bugs.webkit.org/show_bug.cgi?id=216170 about this.
+ * platform/mac-wk1/editing/mac/attributed-string/letter-spacing-expected.txt: Updated.
+ * platform/mac-mojave-wk1/editing/mac/attributed-string/letter-spacing-expected.txt: Updated
+
2020-09-05 Oriol Brufau <obru...@igalia.com>
[css-grid] Use min-content size for intrinsic maximums resolution
Modified: trunk/LayoutTests/TestExpectations (266682 => 266683)
--- trunk/LayoutTests/TestExpectations 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/LayoutTests/TestExpectations 2020-09-06 22:20:54 UTC (rev 266683)
@@ -3309,7 +3309,6 @@
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-family-name-021.xht [ ImageOnlyFailure ]
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-family-name-024.xht [ ImageOnlyFailure ]
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-family-name-025.html [ ImageOnlyFailure ]
-webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-feature-resolution-001.html [ ImageOnlyFailure ]
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html [ ImageOnlyFailure ]
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-003.tentative.html [ ImageOnlyFailure ]
webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-004.tentative.html [ ImageOnlyFailure ]
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (266682 => 266683)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-09-06 22:20:54 UTC (rev 266683)
@@ -1,3 +1,17 @@
+2020-09-06 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Letter-spacing should disable ligatures
+ https://bugs.webkit.org/show_bug.cgi?id=176215
+ <rdar://problem/17044265>
+
+ Reviewed by Antti Koivisto.
+
+ This test is sensitive the order of the CSSPropertyID enum values.
+
+ * web-platform-tests/css/cssom/css-style-attr-decl-block-expected.txt: Apparently this
+ test is sensitive to the ordering of CSSPropertyID enum values in CSSPropertyNames.h.
+ I filed https://bugs.webkit.org/show_bug.cgi?id=216170 about this.
+
2020-09-05 Oriol Brufau <obru...@igalia.com>
[css-grid] Use min-content size for intrinsic maximums resolution
Modified: trunk/LayoutTests/platform/mac-mojave-wk1/editing/mac/attributed-string/letter-spacing-expected.txt (266682 => 266683)
--- trunk/LayoutTests/platform/mac-mojave-wk1/editing/mac/attributed-string/letter-spacing-expected.txt 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/LayoutTests/platform/mac-mojave-wk1/editing/mac/attributed-string/letter-spacing-expected.txt 2020-09-06 22:20:54 UTC (rev 266683)
@@ -23,9 +23,14 @@
HyphenationFactor: 0
TighteningForTruncation: YES
HeaderLevel: 0
-[3pt ]
+[3pt]
NSFont: Times-Roman 16.00 pt.
NSKern: 0pt
NSStrokeColor: #000000 (sRGB)
NSStrokeWidth: 0
+[ ]
+ NSFont: Times-Roman 16.00 pt.
+ NSKern: 0pt
+ NSStrokeColor: #000000 (sRGB)
+ NSStrokeWidth: 0
Modified: trunk/LayoutTests/platform/mac-wk1/editing/mac/attributed-string/letter-spacing-expected.txt (266682 => 266683)
--- trunk/LayoutTests/platform/mac-wk1/editing/mac/attributed-string/letter-spacing-expected.txt 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/LayoutTests/platform/mac-wk1/editing/mac/attributed-string/letter-spacing-expected.txt 2020-09-06 22:20:54 UTC (rev 266683)
@@ -23,9 +23,14 @@
HyphenationFactor: 0
TighteningForTruncation: YES
HeaderLevel: 0 LineBreakStrategy 0
-[3pt ]
+[3pt]
NSFont: Times-Roman 16.00 pt.
NSKern: 0pt
NSStrokeColor: #000000 (sRGB)
NSStrokeWidth: 0
+[ ]
+ NSFont: Times-Roman 16.00 pt.
+ NSKern: 0pt
+ NSStrokeColor: #000000 (sRGB)
+ NSStrokeWidth: 0
Modified: trunk/Source/WebCore/ChangeLog (266682 => 266683)
--- trunk/Source/WebCore/ChangeLog 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/ChangeLog 2020-09-06 22:20:54 UTC (rev 266683)
@@ -1,3 +1,74 @@
+2020-09-06 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Letter-spacing should disable ligatures
+ https://bugs.webkit.org/show_bug.cgi?id=176215
+ <rdar://problem/17044265>
+
+ Reviewed by Antti Koivisto.
+
+ The CSS Text spec[1] says:
+ "When the effective spacing between two characters is not zero (due to either justification
+ or a non-zero value of letter-spacing), user agents should not apply optional ligatures."
+
+ The CSS Fonts spec[2] describes exactly how this is supposed to work:
+ "Step 11. Feature settings determined by properties other than font-variant or
+ font-feature-settings are applied. For example, setting a non-default value for the
+ letter-spacing property disables optional ligatures."
+
+ Disabling ligatures requires modifying font features, which means that the information about
+ whether we should disable them or not needs to be inside the FontDescription. This patch adds
+ a new bit, m_shouldDisableLigaturesForSpacing, to FontDescription. preparePlatformFont()
+ reads this bit and disables ligatures appropriately.
+
+ There's a bit of complexity here because the letter-spacing value itself lies inside the
+ RenderStyle, but the derived bit lies inside the FontDescriptor, which is one reason why
+ this patch migrates letter-spacing to use custom codegen functions. There's also a bit of
+ complexity about dependencies which is explained in a comment in
+ maybeUpdateFontForLetterSpacing().
+
+ [1] https://drafts.csswg.org/css-text-3/#letter-spacing-property
+ [2] https://drafts.csswg.org/css-fonts-4/#feature-variation-precedence
+
+ Test: imported/w3c/web-platform-tests/css/css-fonts/font-feature-resolution-001.html
+
+ * css/CSSProperties.json: letter-spacing has to be high-priority because it affects font
+ selection, but it has to be processed after zoom because its <length> value is sensitive to
+ zoom. This adds a new keyword CSSProperties.json: sink-property which can let a property
+ sink to the bottom of its priority bucket.
+ * css/makeprop.pl:
+ (addProperty):
+ (sortByDescendingPriorityAndName):
+ * platform/graphics/FontCache.h:
+ (WebCore::FontDescriptionKey::makeFlagsKey):
+ * platform/graphics/FontDescription.h:
+ (WebCore::FontDescription::shouldDisableLigaturesForSpacing const):
+ (WebCore::FontDescription::setShouldDisableLigaturesForSpacing):
+ (WebCore::FontDescription::operator== const):
+ (WebCore::FontDescription::encode const):
+ (WebCore::FontDescription::decode):
+ * platform/graphics/cocoa/FontCacheCoreText.cpp:
+ (WebCore::preparePlatformFont): We can get into a situation where "liga" and "clig" don't
+ match, which means whichever one is later clobbers whichever one is earlier when applied to
+ AAT fonts. We need to make sure these values match so we don't get surprising results.
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::setLetterSpacing):
+ (WebCore::RenderStyle::setLetterSpacingWithoutUpdatingFontDescription):
+ * rendering/style/RenderStyle.h:
+ * style/StyleBuilderCustom.h:
+ (WebCore::Style::applyLetterSpacing):
+ (WebCore::Style::BuilderCustom::applyInheritLetterSpacing):
+ (WebCore::Style::BuilderCustom::applyInitialLetterSpacing):
+ (WebCore::Style::maybeUpdateFontForLetterSpacing):
+ (WebCore::Style::BuilderCustom::applyValueLetterSpacing):
+ (WebCore::Style::BuilderCustom::applyValueWebkitLocale):
+ (WebCore::Style::BuilderCustom::applyInitialFontFamily):
+ (WebCore::Style::BuilderCustom::applyInheritFontFamily):
+ (WebCore::Style::BuilderCustom::applyValueFontFamily):
+ (WebCore::Style::BuilderCustom::applyInitialFontSize):
+ (WebCore::Style::BuilderCustom::applyInheritFontSize):
+ (WebCore::Style::BuilderCustom::applyValueFontSize):
+ * style/StyleBuilderState.h:
+
2020-09-06 Zalan Bujtas <za...@apple.com>
[LFC][IFC] LineBox should contain all inline boxes
Modified: trunk/Source/WebCore/css/CSSProperties.json (266682 => 266683)
--- trunk/Source/WebCore/css/CSSProperties.json 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/css/CSSProperties.json 2020-09-06 22:20:54 UTC (rev 266683)
@@ -109,6 +109,11 @@
"",
"* high-priority:",
"Whether the property needs to be applied before non-high-priority properties",
+ "in CSS cascading order. High priority properties must not accept <length>",
+ "values. All font-properties must be high-priority.",
+ "",
+ "* sink-priority:",
+ "Whether the property needs to be applied at the end of its priority bucket",
"in CSS cascading order.",
"",
"* related-property:",
@@ -2671,6 +2676,10 @@
"letter-spacing": {
"inherited": true,
"codegen-properties": {
+ "custom": "All",
+ "font-property": true,
+ "high-priority": true,
+ "sink-priority": true,
"converter": "Spacing"
},
"specification": {
Modified: trunk/Source/WebCore/css/makeprop.pl (266682 => 266683)
--- trunk/Source/WebCore/css/makeprop.pl 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/css/makeprop.pl 2020-09-06 22:20:54 UTC (rev 266683)
@@ -67,6 +67,7 @@
my $numPredefinedProperties = 2;
my %nameIsInherited;
my %nameIsHighPriority;
+my %namePriorityShouldSink;
my %propertiesWithStyleBuilderOptions;
my %styleBuilderOptions = (
"animatable" => 1, # Defined in Source/WebCore/style/StyleBuilderConverter.h
@@ -241,6 +242,8 @@
next;
} elsif ($codegenOptionName eq "high-priority") {
$nameIsHighPriority{$name} = 1;
+ } elsif ($codegenOptionName eq "sink-priority") {
+ $namePriorityShouldSink{$name} = 1;
} elsif ($codegenOptionName eq "related-property") {
$relatedProperty{$name} = $codegenProperties->{"related-property"}
} elsif ($codegenOptionName eq "aliases") {
@@ -278,6 +281,12 @@
if (!!$nameIsHighPriority{$a} > !!$nameIsHighPriority{$b}) {
return -1;
}
+ if (!!$namePriorityShouldSink{$a} < !!$namePriorityShouldSink{$b}) {
+ return -1;
+ }
+ if (!!$namePriorityShouldSink{$a} > !!$namePriorityShouldSink{$b}) {
+ return 1;
+ }
# Sort names without leading '-' to the front
if (substr($a, 0, 1) eq "-" && substr($b, 0, 1) ne "-") {
return 1;
Modified: trunk/Source/WebCore/platform/graphics/FontCache.h (266682 => 266683)
--- trunk/Source/WebCore/platform/graphics/FontCache.h 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/platform/graphics/FontCache.h 2020-09-06 22:20:54 UTC (rev 266683)
@@ -129,7 +129,8 @@
private:
static std::array<unsigned, 2> makeFlagsKey(const FontDescription& description)
{
- unsigned first = static_cast<unsigned>(description.script()) << 14
+ unsigned first = static_cast<unsigned>(description.script()) << 15
+ | static_cast<unsigned>(description.shouldDisableLigaturesForSpacing()) << 14
| static_cast<unsigned>(description.shouldAllowUserInstalledFonts()) << 13
| static_cast<unsigned>(description.fontStyleAxis() == FontStyleAxis::slnt) << 12
| static_cast<unsigned>(description.opticalSizing()) << 11
Modified: trunk/Source/WebCore/platform/graphics/FontDescription.cpp (266682 => 266683)
--- trunk/Source/WebCore/platform/graphics/FontDescription.cpp 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/platform/graphics/FontDescription.cpp 2020-09-06 22:20:54 UTC (rev 266683)
@@ -63,6 +63,7 @@
, m_opticalSizing(static_cast<unsigned>(FontOpticalSizing::Enabled))
, m_fontStyleAxis(FontCascadeDescription::initialFontStyleAxis() == FontStyleAxis::ital)
, m_shouldAllowUserInstalledFonts(static_cast<unsigned>(AllowUserInstalledFonts::No))
+ , m_shouldDisableLigaturesForSpacing(false)
{
}
Modified: trunk/Source/WebCore/platform/graphics/FontDescription.h (266682 => 266683)
--- trunk/Source/WebCore/platform/graphics/FontDescription.h 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/platform/graphics/FontDescription.h 2020-09-06 22:20:54 UTC (rev 266683)
@@ -96,6 +96,7 @@
FontOpticalSizing opticalSizing() const { return static_cast<FontOpticalSizing>(m_opticalSizing); }
FontStyleAxis fontStyleAxis() const { return m_fontStyleAxis ? FontStyleAxis::ital : FontStyleAxis::slnt; }
AllowUserInstalledFonts shouldAllowUserInstalledFonts() const { return static_cast<AllowUserInstalledFonts>(m_shouldAllowUserInstalledFonts); }
+ bool shouldDisableLigaturesForSpacing() const { return m_shouldDisableLigaturesForSpacing; }
void setComputedSize(float s) { m_computedSize = clampToFloat(s); }
void setItalic(Optional<FontSelectionValue> italic) { m_fontSelectionRequest.slope = italic; }
@@ -131,6 +132,7 @@
void setOpticalSizing(FontOpticalSizing sizing) { m_opticalSizing = static_cast<unsigned>(sizing); }
void setFontStyleAxis(FontStyleAxis axis) { m_fontStyleAxis = axis == FontStyleAxis::ital; }
void setShouldAllowUserInstalledFonts(AllowUserInstalledFonts shouldAllowUserInstalledFonts) { m_shouldAllowUserInstalledFonts = static_cast<unsigned>(shouldAllowUserInstalledFonts); }
+ void setShouldDisableLigaturesForSpacing(bool shouldDisableLigaturesForSpacing) { m_shouldDisableLigaturesForSpacing = shouldDisableLigaturesForSpacing; }
static AtomString platformResolveGenericFamily(UScriptCode, const AtomString& locale, const AtomString& familyName);
@@ -174,6 +176,7 @@
unsigned m_opticalSizing : 1; // FontOpticalSizing
unsigned m_fontStyleAxis : 1; // Whether "font-style: italic" or "font-style: oblique 20deg" was specified
unsigned m_shouldAllowUserInstalledFonts : 1; // AllowUserInstalledFonts: If this description is allowed to match a user-installed font
+ unsigned m_shouldDisableLigaturesForSpacing : 1; // If letter-spacing is nonzero, we need to disable ligatures, which affects font preparation
};
inline bool FontDescription::operator==(const FontDescription& other) const
@@ -208,7 +211,8 @@
&& m_variantEastAsianRuby == other.m_variantEastAsianRuby
&& m_opticalSizing == other.m_opticalSizing
&& m_fontStyleAxis == other.m_fontStyleAxis
- && m_shouldAllowUserInstalledFonts == other.m_shouldAllowUserInstalledFonts;
+ && m_shouldAllowUserInstalledFonts == other.m_shouldAllowUserInstalledFonts
+ && m_shouldDisableLigaturesForSpacing == other.m_shouldDisableLigaturesForSpacing;
}
template<class Encoder>
@@ -247,6 +251,7 @@
encoder << opticalSizing();
encoder << fontStyleAxis();
encoder << shouldAllowUserInstalledFonts();
+ encoder << shouldDisableLigaturesForSpacing();
}
template<class Decoder>
@@ -410,6 +415,11 @@
if (!shouldAllowUserInstalledFonts)
return WTF::nullopt;
+ Optional<bool> shouldDisableLigaturesForSpacing;
+ decoder >> shouldDisableLigaturesForSpacing;
+ if (!shouldDisableLigaturesForSpacing)
+ return WTF::nullopt;
+
fontDescription.setFeatureSettings(WTFMove(*featureSettings));
#if ENABLE(VARIATION_FONTS)
fontDescription.setVariationSettings(WTFMove(*variationSettings));
@@ -443,6 +453,7 @@
fontDescription.setOpticalSizing(*opticalSizing);
fontDescription.setFontStyleAxis(*fontStyleAxis);
fontDescription.setShouldAllowUserInstalledFonts(*shouldAllowUserInstalledFonts);
+ fontDescription.setShouldDisableLigaturesForSpacing(*shouldDisableLigaturesForSpacing);
return fontDescription;
}
Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp (266682 => 266683)
--- trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp 2020-09-06 22:20:54 UTC (rev 266683)
@@ -560,6 +560,7 @@
const auto& features = fontDescription.featureSettings();
const auto& variantSettings = fontDescription.variantSettings();
auto textRenderingMode = fontDescription.textRenderingMode();
+ auto shouldDisableLigaturesForSpacing = fontDescription.shouldDisableLigaturesForSpacing();
// We might want to check fontType.trackingType == FontType::TrackingType::Manual here, but in order to maintain compatibility with the rest of the system, we don't.
bool noFontFeatureSettings = features.isEmpty();
@@ -572,7 +573,7 @@
bool variantSettingsIsNormal = variantSettings.isAllNormal();
bool dontNeedToApplyOpticalSizing = fontOpticalSizing == FontOpticalSizing::Enabled && !forceOpticalSizingOn;
bool fontFaceDoesntSpecifyFeatures = !fontFaceFeatures || fontFaceFeatures->isEmpty();
- if (noFontFeatureSettings && noFontVariationSettings && textRenderingModeIsAuto && variantSettingsIsNormal && dontNeedToApplyOpticalSizing && fontFaceDoesntSpecifyFeatures) {
+ if (noFontFeatureSettings && noFontVariationSettings && textRenderingModeIsAuto && variantSettingsIsNormal && dontNeedToApplyOpticalSizing && fontFaceDoesntSpecifyFeatures && !shouldDisableLigaturesForSpacing) {
#if HAVE(CTFONTCREATEFORCHARACTERSWITHLANGUAGEANDOPTION)
return originalFont;
#else
@@ -580,37 +581,53 @@
#endif
}
- // This algorithm is described at http://www.w3.org/TR/css3-fonts/#feature-precedence
+ // This algorithm is described at https://drafts.csswg.org/css-fonts-4/#feature-variation-precedence
FeaturesMap featuresToBeApplied;
+ auto applyFeature = [&](const FontTag& tag, int value) {
+ // AAT doesn't differentiate between liga and clig. We need to make sure they always agree.
+ featuresToBeApplied.set(tag, value);
+ if (fontType.aatShaping) {
+ if (tag == fontFeatureTag("liga"))
+ featuresToBeApplied.set(fontFeatureTag("clig"), value);
+ else if (tag == fontFeatureTag("clig"))
+ featuresToBeApplied.set(fontFeatureTag("liga"), value);
+ }
+ };
+
// Step 1: CoreText handles default features (such as required ligatures).
- // Step 2: Consult with font-variant-* inside @font-face
- // This no longer happens any more: https://github.com/w3c/csswg-drafts/issues/2531
-
- // Step 3: Consult with font-feature-settings inside @font-face
+ // Step 6: Consult with font-feature-settings inside @font-face
if (fontFaceFeatures) {
for (auto& fontFaceFeature : *fontFaceFeatures)
- featuresToBeApplied.set(fontFaceFeature.tag(), fontFaceFeature.value());
+ applyFeature(fontFaceFeature.tag(), fontFaceFeature.value());
}
- // Step 4: Font-variant
+ // Step 10: Font-variant
for (auto& newFeature : computeFeatureSettingsFromVariants(variantSettings))
- featuresToBeApplied.set(newFeature.key, newFeature.value);
+ applyFeature(newFeature.key, newFeature.value);
- // Step 5: Other properties (text-rendering)
+ // Step 11: Other properties
if (textRenderingMode == TextRenderingMode::OptimizeSpeed) {
- featuresToBeApplied.set(fontFeatureTag("liga"), 0);
- featuresToBeApplied.set(fontFeatureTag("clig"), 0);
- featuresToBeApplied.set(fontFeatureTag("dlig"), 0);
- featuresToBeApplied.set(fontFeatureTag("hlig"), 0);
- featuresToBeApplied.set(fontFeatureTag("calt"), 0);
+ applyFeature(fontFeatureTag("liga"), 0);
+ applyFeature(fontFeatureTag("clig"), 0);
+ applyFeature(fontFeatureTag("dlig"), 0);
+ applyFeature(fontFeatureTag("hlig"), 0);
+ applyFeature(fontFeatureTag("calt"), 0);
}
+ if (shouldDisableLigaturesForSpacing) {
+ applyFeature(fontFeatureTag("liga"), 0);
+ applyFeature(fontFeatureTag("clig"), 0);
+ applyFeature(fontFeatureTag("dlig"), 0);
+ applyFeature(fontFeatureTag("hlig"), 0);
+ // Core Text doesn't disable calt when letter-spacing is applied, so we won't either.
+ }
- // Step 6: Font-feature-settings
+ // Step 13: Font-feature-settings
for (auto& newFeature : features)
- featuresToBeApplied.set(newFeature.tag(), newFeature.value());
+ applyFeature(newFeature.tag(), newFeature.value());
+ // The variation fonts are separated from the font features because they don't conflict, and allow us to only have a single ENABLE().
#if ENABLE(VARIATION_FONTS)
VariationsMap variationsToBeApplied;
@@ -624,6 +641,7 @@
variationsToBeApplied.set(tag, valueToApply);
};
+ // Step 2: font-weight, font-stretch, and font-style
// The system font is somewhat magical. Don't mess with its variations.
if (applyWeightWidthSlopeVariations && !fontIsSystemFont(originalFont)) {
float weight = fontSelectionRequest.weight;
@@ -648,6 +666,13 @@
applyVariation({{'s', 'l', 'n', 't'}}, slope);
}
+ // FIXME: Implement Step 5: font-named-instance
+
+ // FIXME: Implement Step 6: the font-variation-settings descriptor inside @font-face
+
+ // FIXME: Move font-optical-sizing handling here. It should be step 9.
+
+ // Step 12: font-variation-settings
for (auto& newVariation : variations)
applyVariation(newVariation.tag(), newVariation.value());
#endif // ENABLE(VARIATION_FONTS)
@@ -678,6 +703,8 @@
}
#endif
+ // Step 9: font-optical-sizing
+ // FIXME: Apply this before font-variation-settings
if (forceOpticalSizingOn || textRenderingMode == TextRenderingMode::OptimizeLegibility) {
#if HAVE(CORETEXT_AUTO_OPTICAL_SIZING)
CFDictionaryAddValue(attributes.get(), kCTFontOpticalSizeAttribute, CFSTR("auto"));
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (266682 => 266683)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-09-06 22:20:54 UTC (rev 266683)
@@ -1816,8 +1816,22 @@
m_rareInheritedData.access().wordSpacing = WTFMove(value);
}
-void RenderStyle::setLetterSpacing(float v) { m_inheritedData.access().fontCascade.setLetterSpacing(v); }
+void RenderStyle::setLetterSpacing(float letterSpacing)
+{
+ FontSelector* currentFontSelector = fontCascade().fontSelector();
+ auto description = fontDescription();
+ description.setShouldDisableLigaturesForSpacing(letterSpacing);
+ setFontDescription(WTFMove(description));
+ fontCascade().update(currentFontSelector);
+ setLetterSpacingWithoutUpdatingFontDescription(letterSpacing);
+}
+
+void RenderStyle::setLetterSpacingWithoutUpdatingFontDescription(float letterSpacing)
+{
+ m_inheritedData.access().fontCascade.setLetterSpacing(letterSpacing);
+}
+
void RenderStyle::setFontSize(float size)
{
// size must be specifiedSize if Text Autosizing is enabled, but computedSize if text
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (266682 => 266683)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2020-09-06 22:20:54 UTC (rev 266683)
@@ -963,7 +963,10 @@
void setWhiteSpace(WhiteSpace v) { m_inheritedFlags.whiteSpace = static_cast<unsigned>(v); }
void setWordSpacing(Length&&);
+
+ // If letter-spacing is nonzero, we disable ligatures, which means this property affects font preparation.
void setLetterSpacing(float);
+ void setLetterSpacingWithoutUpdatingFontDescription(float);
void clearBackgroundLayers() { m_backgroundData.access().background = "" }
void inheritBackgroundLayers(const FillLayer& parent) { m_backgroundData.access().background = "" }
Modified: trunk/Source/WebCore/style/StyleBuilderCustom.h (266682 => 266683)
--- trunk/Source/WebCore/style/StyleBuilderCustom.h 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/style/StyleBuilderCustom.h 2020-09-06 22:20:54 UTC (rev 266683)
@@ -88,6 +88,7 @@
#if ENABLE(CSS_IMAGE_RESOLUTION)
DECLARE_PROPERTY_CUSTOM_HANDLERS(ImageResolution);
#endif
+ DECLARE_PROPERTY_CUSTOM_HANDLERS(LetterSpacing);
#if ENABLE(TEXT_AUTOSIZING)
DECLARE_PROPERTY_CUSTOM_HANDLERS(LineHeight);
#endif
@@ -609,6 +610,61 @@
DEFINE_BORDER_IMAGE_MODIFIER_HANDLER(WebkitMaskBoxImage, Slice)
DEFINE_BORDER_IMAGE_MODIFIER_HANDLER(WebkitMaskBoxImage, Width)
+static inline void applyLetterSpacing(BuilderState& builderState, float letterSpacing)
+{
+ // Setting the letter-spacing from a positive value to another positive value shouldn't require fonts to get updated.
+
+ bool shouldDisableLigaturesForSpacing = letterSpacing;
+ if (shouldDisableLigaturesForSpacing != builderState.fontDescription().shouldDisableLigaturesForSpacing()) {
+ auto fontDescription = builderState.fontDescription();
+ fontDescription.setShouldDisableLigaturesForSpacing(letterSpacing);
+ builderState.setFontDescription(WTFMove(fontDescription));
+ }
+
+ builderState.style().setLetterSpacingWithoutUpdatingFontDescription(letterSpacing);
+}
+
+inline void BuilderCustom::applyInheritLetterSpacing(BuilderState& builderState)
+{
+ applyLetterSpacing(builderState, builderState.parentStyle().letterSpacing());
+}
+
+inline void BuilderCustom::applyInitialLetterSpacing(BuilderState& builderState)
+{
+ applyLetterSpacing(builderState, RenderStyle::initialLetterSpacing());
+}
+
+void maybeUpdateFontForLetterSpacing(BuilderState& builderState, CSSValue& value)
+{
+ // This is unfortunate. It's related to https://github.com/w3c/csswg-drafts/issues/5498.
+ //
+ // From StyleBuilder's point of view, there's a dependency cycle:
+ // letter-spacing accepts an arbitrary <length>, which must be resolved against a font, which must
+ // be selected after all the properties that affect font selection are processed, but letter-spacing
+ // itself affects font selection because it can disable font features. StyleBuilder has some (valid)
+ // ASSERT()s which would fire because of this cycle.
+ //
+ // There isn't *actually* a dependency cycle, though, as none of the font-relative units are
+ // actually sensitive to font features (luckily). The problem is that our StyleBuilder is only
+ // smart enough to consider fonts as one indivisible thing, rather than having the deeper
+ // understanding that different parts of fonts may or may not depend on each other.
+ //
+ // So, we update the font early here, so that if there is a font-relative unit inside the CSSValue,
+ // its font is updated and ready to go. In the worst case there might be a second call to
+ // updateFont() later, but that isn't bad for perf because 1. It only happens twice if there is
+ // actually a font-relative unit passed to letter-spacing, and 2. updateFont() internally has logic
+ // to only do work if the font is actually dirty.
+
+ if (is<CSSPrimitiveValue>(value) && downcast<CSSPrimitiveValue>(value).isFontRelativeLength())
+ builderState.updateFont();
+}
+
+inline void BuilderCustom::applyValueLetterSpacing(BuilderState& builderState, CSSValue& value)
+{
+ maybeUpdateFontForLetterSpacing(builderState, value);
+ applyLetterSpacing(builderState, BuilderConverter::convertSpacing(builderState, value));
+}
+
#if ENABLE(TEXT_AUTOSIZING)
inline void BuilderCustom::applyInheritLineHeight(BuilderState& builderState)
@@ -749,7 +805,7 @@
{
auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
- FontCascadeDescription fontDescription = builderState.style().fontDescription();
+ FontCascadeDescription fontDescription = builderState.fontDescription();
if (primitiveValue.valueID() == CSSValueAuto)
fontDescription.setSpecifiedLocale(nullAtom());
else
@@ -889,7 +945,7 @@
inline void BuilderCustom::applyInitialFontFamily(BuilderState& builderState)
{
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
auto initialDesc = FontCascadeDescription();
// We need to adjust the size to account for the generic family change from monospace to non-monospace.
@@ -905,7 +961,7 @@
inline void BuilderCustom::applyInheritFontFamily(BuilderState& builderState)
{
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
auto parentFontDescription = builderState.parentStyle().fontDescription();
fontDescription.setFamilies(parentFontDescription.families());
@@ -917,7 +973,7 @@
{
auto& valueList = downcast<CSSValueList>(value);
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
// Before mapping in a new font-family property, we should reset the generic family.
bool oldFamilyUsedFixedDefaultSize = fontDescription.useFixedDefaultSize();
@@ -1555,7 +1611,7 @@
inline void BuilderCustom::applyInitialFontSize(BuilderState& builderState)
{
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
float size = Style::fontSizeForKeyword(CSSValueMedium, fontDescription.useFixedDefaultSize(), builderState.document());
if (size < 0)
@@ -1574,7 +1630,7 @@
if (size < 0)
return;
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
fontDescription.setKeywordSize(parentFontDescription.keywordSize());
builderState.setFontSize(fontDescription, size);
builderState.setFontDescription(WTFMove(fontDescription));
@@ -1641,7 +1697,7 @@
inline void BuilderCustom::applyValueFontSize(BuilderState& builderState, CSSValue& value)
{
- auto fontDescription = builderState.style().fontDescription();
+ auto fontDescription = builderState.fontDescription();
fontDescription.setKeywordSizeFromIdentifier(CSSValueInvalid);
float parentSize = builderState.parentStyle().fontDescription().specifiedSize();
Modified: trunk/Source/WebCore/style/StyleBuilderState.h (266682 => 266683)
--- trunk/Source/WebCore/style/StyleBuilderState.h 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Source/WebCore/style/StyleBuilderState.h 2020-09-06 22:20:54 UTC (rev 266683)
@@ -40,7 +40,10 @@
namespace Style {
class Builder;
+class BuilderState;
+void maybeUpdateFontForLetterSpacing(BuilderState&, CSSValue&);
+
struct BuilderContext {
Ref<const Document> document;
const RenderStyle& parentStyle;
@@ -98,6 +101,8 @@
CSSToStyleMap& styleMap() { return m_styleMap; }
private:
+ // See the comment in maybeUpdateFontForLetterSpacing() about why this needs to be a friend.
+ friend void maybeUpdateFontForLetterSpacing(BuilderState&, CSSValue&);
friend class Builder;
void adjustStyleForInterCharacterRuby();
Modified: trunk/Tools/ChangeLog (266682 => 266683)
--- trunk/Tools/ChangeLog 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Tools/ChangeLog 2020-09-06 22:20:54 UTC (rev 266683)
@@ -1,3 +1,14 @@
+2020-09-06 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Letter-spacing should disable ligatures
+ https://bugs.webkit.org/show_bug.cgi?id=176215
+ <rdar://problem/17044265>
+
+ Reviewed by Antti Koivisto.
+
+ * Scripts/webkitpy/style/checkers/jsonchecker.py:
+ (JSONCSSPropertiesChecker.check_codegen_properties):
+
2020-09-04 Alex Christensen <achristen...@webkit.org>
Move PDF heads-up display to UI process on macOS
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker.py (266682 => 266683)
--- trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker.py 2020-09-06 21:32:35 UTC (rev 266682)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker.py 2020-09-06 22:20:54 UTC (rev 266683)
@@ -282,6 +282,7 @@
'font-property': self.validate_boolean,
'getter': self.validate_string,
'high-priority': self.validate_boolean,
+ 'sink-priority': self.validate_boolean,
'initial': self.validate_string,
'internal-only': self.validate_boolean,
'longhands': self.validate_array,
