[webkit-changes] [WebKit/WebKit] 9de002: font shorthand should reset more longhands

Darin Adler noreply at github.com
Fri Nov 4 20:37:41 PDT 2022 

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 9de002d111c74dfffa39582a67d6af89b9abf21c
      https://github.com/WebKit/WebKit/commit/9de002d111c74dfffa39582a67d6af89b9abf21c
  Author: Darin Adler <darin at apple.com>
  Date:   2022-11-04 (Fri, 04 Nov 2022)

  Changed paths:
    M LayoutTests/fast/css/font-shorthand-expected.txt
    M LayoutTests/fast/css/font-systemFontID-parsing-expected.txt
    M LayoutTests/fast/css/font-systemFontID-parsing.html
    M LayoutTests/fast/inspector-support/style-expected.txt
    M LayoutTests/fast/text/font-stretch-parse-expected.txt
    M LayoutTests/fast/text/font-stretch-parse.html
    M LayoutTests/fast/text/font-style-parse-expected.txt
    M LayoutTests/fast/text/font-style-parse.html
    M LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/font-palette-vs-shorthand-expected.txt
    M LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/system-fonts-serialization.tentative-expected.txt
    M LayoutTests/imported/w3c/web-platform-tests/css/cssom/font-shorthand-serialization-expected.txt
    A LayoutTests/platform/win/fast/css/font-shorthand-expected.txt
    A LayoutTests/platform/win/fast/inspector-support/style-expected.txt
    M LayoutTests/svg/css/getComputedStyle-basic-expected.txt
    M Source/WebCore/Sources.txt
    M Source/WebCore/WebCore.xcodeproj/project.pbxproj
    M Source/WebCore/css/CSSFontFace.cpp
    M Source/WebCore/css/CSSFontFaceSet.cpp
    R Source/WebCore/css/CSSFontStyleValue.cpp
    R Source/WebCore/css/CSSFontStyleValue.h
    A Source/WebCore/css/CSSFontStyleWithAngleValue.cpp
    A Source/WebCore/css/CSSFontStyleWithAngleValue.h
    M Source/WebCore/css/CSSFontValue.cpp
    M Source/WebCore/css/CSSFontValue.h
    M Source/WebCore/css/CSSProperties.json
    M Source/WebCore/css/CSSValue.cpp
    M Source/WebCore/css/CSSValue.h
    M Source/WebCore/css/ComputedStyleExtractor.cpp
    M Source/WebCore/css/StyleProperties.cpp
    M Source/WebCore/css/makeprop.pl
    M Source/WebCore/css/parser/CSSPropertyParser.cpp
    M Source/WebCore/css/parser/CSSPropertyParser.h
    M Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp
    M Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.h
    M Source/WebCore/editing/EditingStyle.cpp
    M Source/WebCore/style/StyleBuilderConverter.h
    M Source/WebCore/style/StyleBuilderCustom.h
    M Tools/Scripts/do-webcore-rename

  Log Message:
  -----------
  font shorthand should reset more longhands
https://bugs.webkit.org/show_bug.cgi?id=246912
rdar://101486558

Reviewed by Tim Nguyen.

https://drafts.csswg.org/css-fonts/#font-prop
"All subproperties of the font property are first reset to their initial values"

There are multiple things done here:
1) Add more longhands to font shorthand for all "subproperties of font property" listed.
2) Change style building code to reset them to initial values.
3) Change computed style code to make an empty font shorthand when any of these have non-initial values.
4) Use CSSPrimitiveValue for font-style keywords without angles, rather than CSSFontStyleValue.
5) Renamed CSSFontStyleValue to CSSFontStyleWithAngleValue.

The change (4) simplifies some code. The legacy CSS object model doesn't otherwise represent a
simple keyword as a CSSValue subclass. In an earlier version of this work, the rest of the
changes depended on this, but at this point it's just a separate improvement.

* LayoutTests/fast/css/font-shorthand-expected.txt: Updated since many more properties are set
to initial by the font shorthand now.

* LayoutTests/fast/css/font-systemFontID-parsing-expected.txt: Updated since the test is changed,
but also to expect a wrong result due to a bug we haven't fixed: "font: caption" does not serialize.
* LayoutTests/fast/css/font-systemFontID-parsing.html: Updated to match what's specified, where
font-family and other font subproperties do not reflect system fonts, which can only be specified
with the font shorthand (which makes it not quite a shorthand).

* LayoutTests/fast/inspector-support/style-expected.txt: Updated since many more properties are set
to initial by the font shorthand now.

* LayoutTests/fast/text/font-stretch-parse-expected.txt: Updated.
* LayoutTests/fast/text/font-stretch-parse.html: Updated to expect font shorthand to serialize
as empty string when font-stretch is set to a value font shorthand can't express.

* LayoutTests/fast/text/font-style-parse-expected.txt: Updated.
* LayoutTests/fast/text/font-style-parse.html: Updated to expect font shorthand to serialize
as empty string when font-stretch is set to a value font shorthand can't express. Also took
out "window." so test is slightly less wordy.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/font-palette-vs-shorthand-expected.txt:
Expect all PASS instead of FAIL.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/system-fonts-serialization.tentative-expected.txt:
Expect a different failure; this test expects all subproperties of font to serialize as the
empty string when the font property is set to a system font. This change doesn't do that for
all of them, so the test doesn't pass yet, but it fails later.

* LayoutTests/imported/w3c/web-platform-tests/css/cssom/font-shorthand-serialization-expected.txt:
Updated since the font shorthand now affects so many more font subproperties. The test is still
failing because we haven't added the font shorthand to the StyleProperties::asTextInternal function,
and this will need to remain until we revise editing code to not depend on current behavior.

* LayoutTests/platform/win/fast/css/font-shorthand-expected.txt: Added. This is different because
PLATFORM(WIN) doesn't yet support variation fonts.
* LayoutTests/platform/win/fast/inspector-support/style-expected.txt: Added. Ditto.

* LayoutTests/svg/css/getComputedStyle-basic-expected.txt: Updated since font-style is now a
CSSPrimitiveValue instead of a CSSFontStyleValue, web-exposed in the legacy CSS object model.

* Source/WebCore/Sources.txt: Renamed to CSSFontStyleWithAngleValue.
* Source/WebCore/WebCore.xcodeproj/project.pbxproj: Ditto.

* Source/WebCore/css/CSSFontFace.cpp:
(WebCore::calculateItalicRange): Instead of explicitly checking for CSSFontStyleValue, just call
BuilderConverter::convertFontStyleFromValue on anything other than a CSSFontStyleRangeValue. This
makes this function work with CSSPrimitiveValue, and it's nicer for encapsulation since this
already needs to know about the CSSFontStyleRangeValue class.

* Source/WebCore/css/CSSFontFaceSet.cpp: Removed include of CSSFontStyleValue.h.

* Source/WebCore/css/CSSFontStyleWithAngleValue.cpp: Renamed from Source/WebCore/css/CSSFontStyleValue.cpp.
(WebCore::CSSFontStyleValue::CSSFontStyleValue): Moved out of header.
Also only take a single primitive value for oblique angle.
(WebCore::CSSFontStyleValue::create): Ditto.
(WebCore::CSSFontStyleValue::customCSSText const): Updated since this class is now only used for
oblique with an angle. Also use makeString instead of StringBuilder.
(WebCore::CSSFontStyleValue::equals const): Updated since class is now only used for oblique
with an angle.
* Source/WebCore/css/CSSFontStyleWithAngleValue.h: Renamed from Source/WebCore/css/CSSFontStyleValue.h.
Changed this to a class that just stores oblique with a non-optional angle since we now use
CSSPrimitiveValue directly for keywords without angles. Also removed isItalicOrOblique function,
since this is an editing heuristic that should have been using italicThreshold, not italicValue
anyway. Moved logic into editing code. For coding style reasons, added an obliqueAngle getter
and made data members private.

* Source/WebCore/css/CSSFontValue.cpp: Updated include for CSSFontStyleWithAngleValue change.
* Source/WebCore/css/CSSFontValue.h: Changed style member to CSSValue since it can now be
either a CSSPrimitiveValue or a CSSFontStyleValue.

* Source/WebCore/css/CSSProperties.json: Added all new longhands for all subproperties of
the font shorthand. Used an order that seems logical. Strangely, the order of longhands
is web-exposed so should probably be standardized and tested.

* Source/WebCore/css/CSSValue.cpp: Updated for CSSFontStyleWithAngleValue change.
* Source/WebCore/css/CSSValue.h: Ditto.

* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::fontStyle): Changed return type to CSSValue since this can return a primitive value.
Revised to only return a CSSFontStyleValue when an angle is specified.
(WebCore::fontShorthandValueForSelectionProperties): Deleted. Merged into new fontShorthandValue.
(WebCore::fontShorthandValue): Added. Implements rule for when to compute as empty, when any
longhand property is not expressible by shorthand. This separates those rules from actual
code that builds serializable value, which can now be more straightforward.
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle): Call fontShorthandValue for CSSPropertFont.

* Source/WebCore/css/StyleProperties.cpp:
(WebCore::StyleProperties::fontValue const): Added a FIXME because while there are no tests
covering it, there may need to be some additional work here to make sure that font is serialized
as empty string in specified style in case where font subproperties other than the ones checked
here are set.
(WebCore::StyleProperties::getPropertyCSSValue const): Added code to return nullptr for system
font shorthands. A better solution is likely to use a different CSSValue class for these, but
that will require changes in many other places.
(WebCore::StyleProperties::asTextInternal const): Added more font subproperty longhands and
revise the comment to make it more clear that we do indeed want to support the font shorthand
once we make it so the editing code no longer relies on it.

* Source/WebCore/css/makeprop.pl: Support properties that are not enabled, but are shorthands
for a longhand that is enabled. The example of this right now is font-optical-sizing.

* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeSystemFont): Deleted. Merged into consumeFont.
(WebCore::CSSPropertyParser::consumeFont): Rearranged this function so all parsing comes first,
then all properties are added in a single block at the end. Added some local lambdas so that code
is shorter and much easier to read. Made this single function handle system fonts too so more code
can be shared, including all the new font subproperties.
(WebCore::CSSPropertyParser::parseShorthand): Updated to remove call to consumeSystemFont.

* Source/WebCore/css/parser/CSSPropertyParser.h: Removed consumeSystemFont.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp:
(WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontStyle): Changed return type to CSSValue
since this can return a primitive value. Revised to only return a CSSFontStyleWithAngleValue when
an angle is specified.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.h: Updated return type of consumeFontStyle.

* Source/WebCore/editing/EditingStyle.cpp:
(WebCore::identifierForStyleProperty): Moved italic threshold logic out of CSSFontStyleValue
into this function, where it can be alongside similar bold threshold logic. Rearranged the code
a bit to be tighter, and added a comment.
(WebCore::StyleChange::extractTextStyles): Removed check for CSSValueOblique because
identifierForStyleProperty can never return CSSValueOblique. That was true before these changes
and remains true.

* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertFontStretchFromValue): Updated to handle system
font keyword. Turns out the correct code path was already there and only an assertion needed
to be changed.
(WebCore::Style::BuilderConverter::convertFontStyleFromValue): Updated to handle keywords
specified as CSSPrimitiveValue.

* Source/WebCore/style/StyleBuilderCustom.h:
(WebCore::Style::applyFontStyle): Added. Helper so we don't copy the entire FontDescription
if we aren't changing it.
(WebCore::Style::BuilderCustom::applyInitialFontStyle): Use applyFontStyle.
(WebCore::Style::BuilderCustom::applyValueFontStyle): Ditto.
(WebCore::Style::BuilderCustom::applyValueFontStyle): Updated to handle keywords specified as
CSSPrimitiveValue, retaining support for system font shorthands. Use applyFontStyle.

* Tools/Scripts/do-webcore-rename: Used this for renaming.

Canonical link: https://commits.webkit.org/256349@main

More information about the webkit-changes mailing list