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
layout: Respond to shorthand property requests with real values #31277
layout: Respond to shorthand property requests with real values #31277
Conversation
Can the maintainer approving the workflows on this also kick off a WPT run? |
🔨 Triggering try run (#7826667521) for Linux WPT, MacOS, Windows, Android |
Test results for linux-wpt-layout-2013 from try job (#7826667521): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (19)
|
Test results for linux-wpt-layout-2020 from try job (#7826667521): Flaky unexpected result (16)
Stable unexpected results that are known to be intermittent (17)
Stable unexpected results (10)
|
|
Ive found that there is a generated type LonghandsToSerialize that seem to include the correct logic for only serializing properties of shorthands that are allowed in the shorthands servo/components/style/properties/shorthands/font.mako.rs Lines 219 to 225 in 5facf43
There are also some WPT tests that use shorthands that are failing because the serialization is expected to be returned in a shorter form than what this solution currently generates. Its not clear to me that shorter forms can be generated without knowing which longhand properties were set and which are default? |
|
||
[Can't serialize shorthand set to initial value when some longhand is missing] | ||
expected: FAIL |
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.
This shouldn't be regressing, but it's not your fault, you will need to rebase after #31299
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.
No longer needed
Signed-off-by: Sebastian C <sebsebmc@gmail.com>
Signed-off-by: Sebastian C <sebsebmc@gmail.com>
Signed-off-by: Sebastian C <sebsebmc@gmail.com>
Signed-off-by: Sebastian C <sebsebmc@gmail.com>
e649f25
to
5ca70a1
Compare
🔨 Triggering try run (#7849202580) for Linux WPT legacy-layout, Linux WPT layout-2020 |
Test results for linux-wpt-layout-2020 from try job (#7849202580): Flaky unexpected result (6)
Stable unexpected results that are known to be intermittent (20)
Stable unexpected results (10)
|
|
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.
You need to update the test expectations.
So if I get it right, the regressions are:
/html/rendering/non-replaced-elements/the-hr-element-0/hr.html
, addressed by <hr> elements are expected to have a default overflow:hidden #31297/css/css-fonts/font-shorthand-serialization-prevention.html
because servo doesn't supportall-small-caps
/css/css-backgrounds/animations/background-position-interpolation.html
It would be good to land #31297 first and investigate what's happening with background-position-interpolation.html
, but otherwise this looks good.
|
||
[Can't serialize shorthand set to initial value when some longhand is missing] | ||
expected: FAIL |
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.
No longer needed
|
No, the test fails because Servo doesn't support Therefore, the assignment fails, and the longhands can be represented by a shorthand value, so the shorthand serializes. |
|
…o#31277) * Respond to shorthand property requests with real values Signed-off-by: Sebastian C <sebsebmc@gmail.com> * Cleanup formatting and old comments Signed-off-by: Sebastian C <sebsebmc@gmail.com> * Update WPT expectations Signed-off-by: Sebastian C <sebsebmc@gmail.com> * Refactor out helper fn Signed-off-by: Sebastian C <sebsebmc@gmail.com> * Cleanup Signed-off-by: Sebastian C <sebsebmc@gmail.com> --------- Signed-off-by: Sebastian C <sebsebmc@gmail.com>
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
…o#31277) (full) {"fail_fast": false, "matrix": [{"name": "Linux WPT", "workflow": "linux", "wpt_layout": "all", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "MacOS", "workflow": "macos", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Windows", "workflow": "windows", "wpt_layout": "none", "profile": "release", "unit_tests": true, "wpt_tests_to_run": ""}, {"name": "Android", "workflow": "android", "wpt_layout": "none", "profile": "release", "unit_tests": false, "wpt_tests_to_run": ""}]}
This ports servo#31277 (with the changes from servo#32066) into legacy layout. Otherwise, turning white-space into a shorthand (servo#32146) would fail some tests that expect the property to be serializable.
Instead of responding with an empty string for shorthand properties, use the mapped longhand properties to respond to computed style requests.
./mach build -d
does not report any errors./mach test-tidy
does not report any errors/css/css-flexbox/parsing/flex-computed.html
,/css/css-flexbox/parsing/flex-flow-computed.html
and/css/css-animations/computed-style-animation-parsing.html