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

Don't trim leading whitespace of anonymous table cells #31803

Merged
merged 1 commit into from Mar 21, 2024
Merged

Don't trim leading whitespace of anonymous table cells #31803

merged 1 commit into from Mar 21, 2024

Conversation

Loirooriol
Copy link
Contributor

A sequence of whitespace shouldn't generate an anonymous table row/cell, but we can't just throw away the leading whitespace, because afterwards we may encounter some other content, and then the leading whitespace should appear in the cell (noticeable with e.g. white-space: pre).

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

@Loirooriol Loirooriol added A-layout/table A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 T-linux-wpt-2020 Do a try run of the WPT labels Mar 21, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Mar 21, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#8371862648) for Linux WPT

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (18)
  • CRASH [expected PASS] /_webgl/conformance/glsl/bugs/long-expressions-should-not-crash.html (#19221)
  • 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 '351 398' over '501 550'
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '500' over '400 425'
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '340 398' over '501 550'
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '501 550' over '502 560'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'normal' over 'oblique 0deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'oblique 0deg' over 'oblique 10deg 40deg'
  • OK /css/cssom-view/matchMedia.html (#31428)
    • PASS [expected FAIL] subtest: iframe.matchMedia("(min-width: 150px)") matches
  • TIMEOUT [expected OK] /fetch/api/redirect/redirect-keepalive.https.any.html
    • TIMEOUT [expected PASS] subtest: [keepalive][iframe][load] mixed content redirect; setting up

      Test timed out

  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes
  • 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 form submission
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • FAIL [expected PASS] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src=''

      assert_unreached: load should not be fired Reached unreachable code

  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-referrer.window.html (#29081)
    • PASS [expected TIMEOUT] subtest: no-referrer referrer policy used to create the starting page
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin.window.html (#29049)
    • PASS [expected FAIL] subtest: Same-origin navigation started from unload handler must be ignored
  • OK /html/browsers/history/the-history-interface/traverse_the_history_2.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • TIMEOUT [expected OK] /html/browsers/history/the-location-interface/assign-replace-from-iframe.html (#31638)
  • FAIL [expected CRASH] /html/canvas/element/manual/text/canvas.2d.disconnected.html (#30063)
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • PASS [expected FAIL] subtest: multipart/form-data: backslash in name (normal form)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: 0x00 in name (formdata event)
  • TIMEOUT /resource-timing/test_resource_timing.html (#25720)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)

      assert_equals: expected 11840256 but got 11840000

  • OK /xhr/send-redirect.htm
    • FAIL [expected PASS] subtest: XMLHttpRequest: send() - Redirects (basics) (307 does redirect)

      assert_equals: expected (string) "GET" but got (object) null

Stable unexpected results that are known to be intermittent (19)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • PASS [expected FAIL] /css/css-sizing/dynamic-available-size-iframe.html (#31559)
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • PASS [expected FAIL] subtest: listeners are called correct number of times
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • PASS [expected TIMEOUT] subtest: background-image sec-fetch-site - HTTPS downgrade (header not sent)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • FAIL [expected PASS] subtest: load event does not fire on window.open('about:blank')

      assert_unreached: load should not be fired Reached unreachable code

  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • FAIL [expected PASS] subtest: Cross-origin navigation started from unload handler must be ignored

      promise_test: Unhandled rejection with value: object "SecurityError: The operation is insecure."

  • PASS [expected CRASH] /html/canvas/element/manual/drawing-text-to-the-canvas/canvas.2d.disconnected-font-size-math.html (#30063)
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • 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"

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/sandbox-top-navigation-cross-site.tentative.sub.window.html (#31754)
    • TIMEOUT [expected FAIL] subtest: A cross-site unsandboxed iframe navigation consumes user activation and disallows top-level navigation.

      Test timed out

    • NOTRUN [expected FAIL] subtest: A same-site unsandboxed iframe navigation does not consume user activation and allows top-level navigation.
    • NOTRUN [expected FAIL] subtest: A same-site unsandboxed iframe navigation without sticky user activation does not allow top-level navigation.
  • OK [expected CRASH] /html/semantics/forms/the-fieldset-element/disabled-003.html (#31730)
  • OK [expected ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • TIMEOUT [expected OK] /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent, even after history navigations by the parent

      Test timed out

    • NOTRUN [expected PASS] subtest: Test that iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that iframe refreshes are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe refreshes are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe refreshes are not observable by the parent
  • TIMEOUT /resource-timing/test_resource_timing.https.html (#25216)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)
  • OK [expected TIMEOUT] /webmessaging/with-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
  • OK [expected TIMEOUT] /webmessaging/without-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank
Stable unexpected results (3)
  • PASS [expected FAIL] /css/css-tables/anonymous-table-ws-001.html
  • OK /css/css-tables/table-model-fixup.html
    • PASS [expected FAIL] subtest: 1.4. Anonymous inline boxes which contains only white space and are between two immediate siblings *each* of which is a table-non-root element, are treated as if they had display: none.
  • PASS [expected FAIL] /css/css-tables/whitespace-001.html

@github-actions GitHub Actions
Copy link

⚠️ Try run (#8371862648) failed.

@Loirooriol Loirooriol force-pushed the anon-table-content-white-space branch from 003285b to 1ffe07b Compare March 21, 2024 10:00
@Loirooriol Loirooriol marked this pull request as ready for review March 21, 2024 10:01
mrobinson
mrobinson approved these changes Mar 21, 2024
View reviewed changes
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.

Nice fix!

components/layout_2020/table/construct.rs Outdated Show resolved Hide resolved
@Loirooriol
Don't trim leading whitespace of anonymous table cells
174284e 
A sequence of whitespace shouldn't generate an anonymous table row/cell,
but we can't just throw away the leading whitespace, because afterwards
we may encounter some other content, and then the leading whitespace
should appear in the cell (noticeable with e.g. `white-space: pre`).
@Loirooriol Loirooriol force-pushed the anon-table-content-white-space branch from 1ffe07b to 174284e Compare March 21, 2024 11:18
@Loirooriol Loirooriol added this pull request to the merge queue Mar 21, 2024
Merged via the queue into servo:main with commit ecabdc2 Mar 21, 2024
9 checks passed
@Loirooriol Loirooriol deleted the anon-table-content-white-space branch March 21, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson approved these changes

Assignees
No one assigned
Labels
A-layout/table A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
Layout 2020: Tables
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Loirooriol @mrobinson