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.

Sign up for GitHub

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

Jump to bottom

layout: align-content with default value normal should behave as strech in flex container #35178

Merged
merged 1 commit into from
Feb 3, 2025
Merged

layout: align-content with default value normal should behave as strech in flex container #35178

merged 1 commit into from
Feb 3, 2025

Conversation

rayguo17
Copy link
Contributor

@rayguo17 rayguo17 commented Jan 27, 2025 •
edited
Loading

According to the spec,
align-content default value normal should have the same behaviour as value stretch in flex container. cc @xiaochengh

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because ___

@jschwe jschwe added the T-linux-wpt-2020 Do a try run of the WPT label Jan 27, 2025
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Jan 27, 2025
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#12988342965) for Linux (WPT)

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (15)
  • TIMEOUT /FileAPI/BlobURL/cross-partition-navigation.https.html (#35133)
    • PASS [expected FAIL] subtest: Blob URL should partition subframe navigation.
  • OK /css/css-grid/alignment/grid-content-alignment-with-abspos-001.html (#34339)
    • FAIL [expected PASS] subtest: .grid 1

      assert_equals: 
<div class="grid" data-expected-width="800" data-expected-height="600">
    <div class="a" id="item" data-offset-x="329" data-offset-y="269" data-expected-width="142" data-expected-height="62" style="place-self: center;"></div>
  </div>
offsetLeft expected 329 but got 0

  • PASS [expected FAIL] /css/css-position/sticky/position-sticky-left-002.html (#35135)
  • PASS [expected FAIL] /css/css-position/sticky/position-sticky-left-003.html
  • PASS [expected FAIL] /css/css-tables/table-cell-overflow-auto-scrolled.html (#35011)
  • OK /css/css-values/cap-invalidation.html (#32757)
    • FAIL [expected PASS] subtest: CSS Values and Units Test: cap invalidation

      uncaught exception: Error: assert_not_equals: expect the capital height of Ahem and sans-serif to be different got disallowed value 371.3333333333333

  • TIMEOUT [expected OK] /custom-elements/reactions/customized-builtins/HTMLMediaElement.html (#31014)
  • ERROR [expected OK] /fetch/fetch-later/quota/same-origin-iframe/empty-payload.tentative.https.window.html (#35176)
  • OK /fetch/private-network-access/worker-blob-fetch.tentative.window.html (#30064)
    • PASS [expected FAIL] subtest: public to public: success.
  • 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')
  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
  • OK /html/browsers/browsing-the-web/overlapping-navigations-and-traversals/cross-document-nav-cross-document-nav.html (#29181)
    • PASS [expected FAIL] subtest: cross-document navigation then cross-document navigation
  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • CRASH [expected OK] /html/canvas/offscreen/canvas-host/2d.canvas.host.size.large.worker.html (#30164)
  • OK /html/semantics/scripting-1/the-script-element/module/dynamic-import/blob-url.any.worker.html (#33909)
    • FAIL [expected PASS] subtest: Revoking a blob URL immediately after calling import will not fail

      promise_test: Unhandled rejection with value: object "TypeError: Dynamic import failed"

Stable unexpected results that are known to be intermittent (9)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • PASS [expected TIMEOUT] subtest: Fetching a blob URL immediately before revoking it works in <script> tags.
  • FAIL [expected PASS] /css/compositing/mix-blend-mode/mix-blend-mode-video-sibling.html (#32849)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • PASS [expected FAIL] subtest: Navigating to a different document with link click
  • OK /html/browsers/history/the-history-interface/traverse_the_history_3.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 3 but got 1 (expected array [6, 3] got [6, 1])

  • PASS [expected FAIL] /html/canvas/element/manual/text/canvas.2d.disconnected.html (#30063)
  • 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_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"

  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)

@github-actions GitHub Actions
Copy link

✨ Try run (#12988342965) succeeded.

Loirooriol
Loirooriol reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

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

Looks good, but maybe you could make it so that resolved_align_content becomes AlignFlags::STRETCH for normal? With a name starting with "resolved" I would expect it to resolve normal.

Something like

        let resolved_align_content: AlignFlags = {
            // Computed value from the style system
            let align_content_style = flex_context.config.align_content.0.primary();
            let mut is_safe = align_content_style.flags() == AlignFlags::SAFE;

            // From https://drafts.csswg.org/css-align/#distribution-flex
            // > `normal` behaves as `stretch`.
            let mut resolved_align_content = match align_content_style.value() {
                AlignFlags::NORMAL => AlignFlags::STRETCH,
                align_content => align_content,
            };

@rayguo17 rayguo17 force-pushed the align-content-flexbox-normal branch from ff9e839 to c94c924 Compare February 3, 2025 03:21
@rayguo17 rayguo17 requested a review from Loirooriol February 3, 2025 07:23
Loirooriol
Loirooriol approved these changes Feb 3, 2025
View reviewed changes
@rayguo17 rayguo17 force-pushed the align-content-flexbox-normal branch from 0dd0041 to c320a5f Compare February 3, 2025 07:58
@Loirooriol Loirooriol enabled auto-merge February 3, 2025 08:12
@Loirooriol Loirooriol added this pull request to the merge queue Feb 3, 2025
Merged via the queue into servo:main with commit 2030b7a Feb 3, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Loirooriol Loirooriol Loirooriol approved these changes

@mrobinson mrobinson Awaiting requested review from mrobinson mrobinson is a code owner

@nicoburns nicoburns Awaiting requested review from nicoburns nicoburns is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rayguo17 @Loirooriol @jschwe