-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Implement math-depth. #52063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement math-depth. #52063
Conversation
|
EWS run on previous version of this PR (hash 4d58d2b) Details |
|
EWS run on previous version of this PR (hash f3edbe9) Details |
|
EWS run on previous version of this PR (hash 8f1eefe) Details |
|
EWS run on previous version of this PR (hash af52eb9) Details |
|
EWS run on previous version of this PR (hash 50874d5) Details |
|
EWS run on previous version of this PR (hash c8fff69) Details |
|
EWS run on previous version of this PR (hash cfcb7c3) Details |
|
EWS run on previous version of this PR (hash 0ef8226) Details |
|
EWS run on previous version of this PR (hash 1809f2e) Details |
|
EWS run on previous version of this PR (hash c8c4b0b) Details |
weinig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, but I like to hear @anttijk's thoughts on the top-priority part.
| "animation-wrapper": "StyleTypeWrapper", | ||
| "settings-flag": "cssMathDepthEnabled", | ||
| "style-converter": "StyleType<MathDepth>", | ||
| "top-priority": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like @anttijk to weigh in on what the costs of adding a new top-priority property might be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks. For what is worth, Chromium does this too, since this property needs to resolve before the font-size one: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_properties.json5;l=1747?q=%22math-depth%22%20file:properties.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we have to do that, Stylo (used by Firefox/Servo) does that too: https://searchfox.org/firefox-main/rev/8e5d58cfed616cb90586c614e53d8ab1ffc8af27/servo/components/style/properties/data.py#55
In practice math-depth is only used for MathML subtrees, so that shouldn't impact the performance when MathML is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is really much cost.
|
EWS run on previous version of this PR (hash 047ec3b) Details |
|
EWS run on previous version of this PR (hash b22a077) Details |
|
EWS run on previous version of this PR (hash 788016c) Details |
|
EWS run on current version of this PR (hash 7cf5016) Details |
https://bugs.webkit.org/show_bug.cgi?id=300967 Reviewed by Antti Koivisto and Sam Weinig. Parse and implement the `math-depth` property defined on MathML Core: https://w3c.github.io/mathml-core/#propdef-math-depth. This is gated behind the `CSSMathDepth` preference. At the moment the property doesn't do anything, a followup patch for `font-size: math` is required. * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/attribute-mapping-002-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/parsing/math-depth-computed-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/parsing/math-depth-valid-expected.txt: * LayoutTests/platform/glib/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/glib/imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative-expected.txt: Removed. * LayoutTests/platform/glib/imported/w3c/web-platform-tests/mathml/relations/css-styling/attribute-mapping-002-expected.txt: * LayoutTests/platform/ios/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/ios/imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative-expected.txt: Removed. * LayoutTests/platform/ios/imported/w3c/web-platform-tests/mathml/relations/css-styling/attribute-mapping-002-expected.txt: Removed. * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/platform/mac/imported/w3c/web-platform-tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative-expected.txt: Removed. * LayoutTests/platform/mac/imported/w3c/web-platform-tests/mathml/relations/css-styling/attribute-mapping-002-expected.txt: Removed. * Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml: * Source/WebCore/Headers.cmake: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/CSSValueKeywords.in: * Source/WebCore/css/mathml.css: (math): (mfrac > *): (mroot > :not(:first-child)): (msub > :not(:first-child),): (mroot > *:last-child): Deleted. (msub > * + *, msup > * + *, msubsup > * + *, mmultiscripts > * + *, munder > * + *, mover > * + *, munderover > * + *): Deleted. * Source/WebCore/css/parser/CSSParserContext.cpp: (WebCore::applyUASheetBehaviorsToContext): * Source/WebCore/rendering/style/RenderStyle.h: * Source/WebCore/rendering/style/RenderStyleInlines.h: (WebCore::RenderStyle::initialMathDepth): (WebCore::RenderStyle::mathDepth const): * Source/WebCore/rendering/style/RenderStyleSetters.h: (WebCore::RenderStyle::setMathDepth): * Source/WebCore/rendering/style/StyleRareInheritedData.cpp: (WebCore::StyleRareInheritedData::StyleRareInheritedData): (WebCore::StyleRareInheritedData::operator== const): (WebCore::StyleRareInheritedData::dumpDifferences const): * Source/WebCore/rendering/style/StyleRareInheritedData.h: * Source/WebCore/style/values/math/StyleMathDepth.cpp: Added. (WebCore::Style::CSSValueConversion<MathDepth>::operator): * Source/WebCore/style/values/math/StyleMathDepth.h: Added. Canonical link: https://commits.webkit.org/302116@main
7cf5016 to
11f6206
Compare
|
Committed 302116@main (11f6206): https://commits.webkit.org/302116@main Reviewed commits have been landed. Closing PR #52063 and removing active labels. |
🧪 gtk-wk2
🛠 ios
11f6206
7cf5016
🛠 playstation🛠 mac-safer-cpp