Skip to content
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

Fix vertical alignment at the root of an IFC #31636

Merged
merged 1 commit into from Mar 13, 2024

Conversation

Loirooriol
Copy link
Contributor

At the root of an inline formatting context, we used its vertical-align in order to compute the strut. That was wrong, since vertical-align on a block container shouldn't affect the contents, it should only affect the alignment of the block container (if it's inline-level) within the parent IFC.

This was only working well if the block container was block-level, since effective_vertical_align_for_inline_layout returned baseline for block-level boxes.

Instead of the outer display type, this patch changes the logic to check whether we are at the root of the IFC.


  • There are tests for these changes

@Loirooriol Loirooriol added A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 T-linux-wpt-2013 Do a try run of the WPT (legacy layout) T-linux-wpt-2020 Do a try run of the WPT labels Mar 13, 2024
@github-actions github-actions bot removed T-linux-wpt-2013 Do a try run of the WPT (legacy layout) T-linux-wpt-2020 Do a try run of the WPT labels Mar 13, 2024
Copy link

🔨 Triggering try run (#8257712869) for Linux WPT

@servo-wpt-sync
Copy link
Collaborator

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#45074) with upstreamable changes.

Copy link

⚠️ Try run (#8257712869) failed.

@Loirooriol Loirooriol force-pushed the vertical-align branch 4 times, most recently from b5ff06c to 96978e5 Compare March 13, 2024 08:25
@Loirooriol Loirooriol added T-linux-wpt-2013 Do a try run of the WPT (legacy layout) T-linux-wpt-2020 Do a try run of the WPT labels Mar 13, 2024
@servo-wpt-sync
Copy link
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#45074).

@github-actions github-actions bot removed T-linux-wpt-2013 Do a try run of the WPT (legacy layout) T-linux-wpt-2020 Do a try run of the WPT labels Mar 13, 2024
Copy link

🔨 Triggering try run (#8261596814) for Linux WPT

Copy link

Test results for linux-wpt-layout-2013 from try job (#8261596814):

Flaky unexpected result (12)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • FAIL [expected PASS] /css/css-text/line-breaking/segment-break-transformation-unremovable-1.html (#28758)
  • OK /css/cssom-view/MediaQueryList-extends-EventTarget.html (#25269)
    • FAIL [expected PASS] subtest: listeners for "change" type are called assert_equals: expected 1 but got 0
  • TIMEOUT /fetch/api/basic/keepalive.any.html (#29536)
    • TIMEOUT [expected PASS] subtest: [keepalive] simple GET request on 'load' [no payload]; setting up Test timed out
    • NOTRUN [expected TIMEOUT] subtest: [keepalive] simple GET request on 'pagehide' [no payload]; setting up
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • PASS [expected FAIL] subtest: load event does not fire on window.open('about:blank')
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK /html/semantics/embedded-content/the-img-element/non-active-document.html (#21544)
    • FAIL [expected PASS] subtest: DOMParser assert_unreached: got unexpected load event Reached unreachable code
    • FAIL [expected PASS] subtest: createHTMLDocument assert_unreached: got unexpected error event Reached unreachable code
    • FAIL [expected PASS] subtest: <template> assert_unreached: got unexpected error event Reached unreachable code
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
    • TIMEOUT [expected FAIL] subtest: Fulfillment handler on pending-then-fulfilled promise Test timed out
    • TIMEOUT [expected FAIL] subtest: Rejection handler on pending-then-rejected promise Test timed out
  • ERROR /resource-timing/content-type-parsing.html (#29131)
    • FAIL [expected TIMEOUT] subtest: mime-type 16 : text/html;charset=�gbk assert_equals: expected (string) "text/html" but got (undefined) undefined
    • TIMEOUT [expected NOTRUN] subtest: mime-type 17 : text/html;charset= gbk Test timed out
  • TIMEOUT /resource-timing/test_resource_timing.https.html (#25216)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img) assert_equals: expected 6450176 but got 6450688
  • OK /workers/WorkerGlobalScope-close.html (#23064)
    • PASS [expected FAIL] subtest: Test sending a message after closing.
Stable unexpected results that are known to be intermittent (13)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/cssom-view/scroll-behavior-smooth-navigation.html (#29564)
    • FAIL [expected PASS] subtest: Smooth scrolling while doing history navigation. assert_not_equals: Shouldn't be scrolled back to top yet. got disallowed value 0
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src=''
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • PASS [expected FAIL] subtest: Cross-origin navigation started from unload handler must be ignored
  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])
  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected TIMEOUT] subtest: opener of discarded auxiliary browsing context assert_object_equals: property "get" expected function "function opener() {
      [native code]
      }" got function "function opener() {
      [native code]
      }"
  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
  • OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK [expected ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • OK [expected TIMEOUT] /resource-timing/nested-context-navigations-iframe.html (#24311)
    • PASS [expected NOTRUN] subtest: Test that iframe navigations are not observable by the parent
    • PASS [expected NOTRUN] subtest: Test that crossorigin iframe navigations are not observable by the parent
    • PASS [expected NOTRUN] subtest: Test that cross-site iframe navigations are not observable by the parent
    • PASS [expected NOTRUN] subtest: Test that iframe refreshes are not observable by the parent
    • PASS [expected NOTRUN] subtest: Test that crossorigin iframe refreshes are not observable by the parent
    • PASS [expected NOTRUN] subtest: Test that cross-site iframe refreshes are not observable by the parent
  • OK [expected TIMEOUT] /webmessaging/without-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:

Copy link

Test results for linux-wpt-layout-2020 from try job (#8261596814):

Flaky unexpected result (14)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '400' should prefer '500' over '350 399'
    • PASS [expected FAIL] subtest: Matching font-weight: '500' should prefer '500' over '450 460'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '350 399' over '340 360'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '200 300' over '400'
    • PASS [expected FAIL] subtest: Matching font-stretch: '110%' should prefer '110% 120%' over '115% 116%'
    • PASS [expected FAIL] subtest: Matching font-stretch: '90%' should prefer '60% 70%' over '110% 140%'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'normal' over 'oblique 0deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'italic' should prefer 'oblique 40deg 50deg' over 'oblique 5deg 10deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 21deg' should prefer 'oblique 21deg' over 'oblique 30deg 60deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -21deg' should prefer 'oblique -10deg' over 'italic'
  • OK /css/cssom-view/scroll-behavior-smooth-navigation.html (#29564)
    • FAIL [expected PASS] subtest: Smooth scrolling while doing history navigation. assert_not_equals: Shouldn't be scrolled back to top yet. got disallowed value 0
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • FAIL [expected PASS] subtest: 0041 set in href="" targeting a frame and clicked assert_equals: expected "A" but got ""
  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected TIMEOUT] subtest: opener of discarded auxiliary browsing context assert_object_equals: property "get" expected function "function opener() {
      [native code]
      }" got function "function opener() {
      [native code]
      }"
  • TIMEOUT [expected OK] /html/canvas/element/path-objects/2d.path.roundrect.1.radius.dompoint.single argument.html
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • OK /html/semantics/embedded-content/the-img-element/non-active-document.html (#21544)
    • FAIL [expected PASS] subtest: DOMParser assert_unreached: got unexpected load event Reached unreachable code
    • FAIL [expected PASS] subtest: createHTMLDocument assert_unreached: got unexpected error event Reached unreachable code
    • FAIL [expected PASS] subtest: <template> assert_unreached: got unexpected error event Reached unreachable code
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: \r\n in value (formdata event)
    • PASS [expected FAIL] subtest: text/plain: non-ASCII in name and value (normal form)
  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • PASS [expected FAIL] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document
  • TIMEOUT /resource-timing/test_resource_timing.https.html (#25216)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img) assert_equals: expected 15450880 but got 15451136
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out
Stable unexpected results that are known to be intermittent (12)
  • FAIL [expected PASS] /_mozilla/mozilla/iframe/resize_after_load.html (#13573)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/cssom-view/MediaQueryList-addListener-handleEvent.html (#24571)
    • FAIL [expected PASS] subtest: calls handleEvent method of event listener assert_equals: expected (object) object "[object Object]" but got (undefined) undefined
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
  • OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: single quote in name (normal form)
  • OK [expected ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • OK [expected TIMEOUT] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
  • OK [expected TIMEOUT] /webmessaging/with-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
Stable unexpected results (1)
  • PASS [expected FAIL] /css/CSS2/linebox/inline-formatting-context-011.xht

Copy link

⚠️ Try run (#8261596814) failed.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

components/layout_2020/flow/inline.rs Outdated Show resolved Hide resolved
At the root of an inline formatting context, we used its vertical-align
in order to compute the strut. That was wrong, since vertical-align
on a block container shouldn't affect the contents, it should only
affect the alignment of the block container (if it's inline-level)
within the parent IFC.

This was only working well if the block container was block-level, since
effective_vertical_align_for_inline_layout returned `baseline` for
block-level boxes.

Instead of the outer display type, this patch changes the logic to check
whether we are at the root of the IFC.
@Loirooriol Loirooriol marked this pull request as ready for review March 13, 2024 10:01
@servo-wpt-sync
Copy link
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#45074).

@Loirooriol Loirooriol added this pull request to the merge queue Mar 13, 2024
Merged via the queue into servo:main with commit 0860deb Mar 13, 2024
9 checks passed
@Loirooriol Loirooriol deleted the vertical-align branch March 13, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
3 participants