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

Fix table track offsets when there is visibility: collapse #32469

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Fix table track offsets when there is visibility: collapse #32469

merged 1 commit into from
Jun 11, 2024

Conversation

Loirooriol
Copy link
Contributor

Each non-collapsed track used to increase the offset by the subsequent border spacing. Now they will take care of their preceding spacing instead.

This way, if a cell spans two rows, and the second is collapsed, the cell won't be forced to be at least as tall as the border spacing. This matches Gecko and Blink (WebKit lacks visibility: collapse).

This makes visibility-collapse-border-spacing-001.html fail because we generate outlines in a different way than Blink. Gecko also fails it in a similar (but different) way.

@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 Jun 10, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Jun 10, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#9452090865) for Linux WPT

@servo-wpt-sync servo-wpt-sync mentioned this pull request Jun 10, 2024
Merged
@servo-wpt-sync
Copy link
Collaborator

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

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (19)
  • CRASH [expected PASS] /_webgl/conformance/glsl/bugs/long-expressions-should-not-crash.html (#19221)
  • TIMEOUT [expected OK] /css/css-transitions/disconnected-element-001.html (#32275)
    • TIMEOUT [expected PASS] subtest: Transitions are canceled when an element is re-parented to the same node

      Test timed out

  • TIMEOUT [expected OK] /css/css-transitions/non-rendered-element-002.html (#32338)
    • TIMEOUT [expected PASS] subtest: Transitions on ::before/::after pseudo-elements are canceled when the content property is cleared

      Test timed out

  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • FAIL [expected PASS] subtest: Check execution order from nested timeout

      assert_equals: Expected nested setTimeout to run second expected true but got false

    • FAIL [expected PASS] subtest: Check execution order on load handler

      assert_equals: Expected onload to run first expected false but got true

  • 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/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • FAIL [expected PASS] subtest: aElement.click() before the load event must NOT replace

      assert_equals: expected "http://web-platform.test:8000/common/blank.html?thereplacement" but got "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/resources/code-injector.html?pipe=sub(none)&code=%0A%20%20%20%20const%20a%20%3D%20document.createElement(%22a%22)%3B%0A%20%20%20%20a.href%20%3D%20%22%2Fcommon%2Fblank.html%3Fthereplacement%22%3B%0A%20%20%20%20document.currentScript.before(a)%3B%0A%20%20%20%20a.click()%3B%0A%20%20"

  • OK /html/browsers/history/the-history-interface/traverse_the_history_3.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
  • TIMEOUT [expected PASS] /html/canvas/element/manual/drawing-text-to-the-canvas/canvas.2d.disconnected-font-size-math.html (#30063)
  • ERROR /html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference.html (#16179)
    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) IMG usemap="no-hash-id"

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • FAIL [expected PASS] subtest: HTML (quirks) OBJECT usemap=undefined

      assert_not_equals: sanity check (too many tests to fit in viewport?) got disallowed value null

    • And 27 more unexpected results...
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
  • 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

  • OK /html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-quirks-mode.html (#21666)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e38 50w, /images/green-16x16.png?e38 51w" sizes="(min-width:calc(0)) 1px"> ref sizes="1px" (quirks mode)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: 0x00 in name (formdata event)
    • PASS [expected FAIL] subtest: text/plain: \r\n in name (formdata event)
  • TIMEOUT [expected CRASH] /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/ignore-opens-during-unload.window.html (#21444)
  • 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
  • OK /html/webappapis/update-rendering/child-document-raf-order.html
    • PASS [expected FAIL] subtest: Ordering of steps in "Update the Rendering" - child document requestAnimationFrame order
  • CRASH [expected PASS] /streams/readable-streams/crashtests/strategy-worker-terminate.html
  • 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 (12)
  • FAIL [expected PASS] /_mozilla/css/iframe/hide_and_show.html (#15265)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • FAIL [expected PASS] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique -5deg' over 'oblique -1deg 0deg'

      assert_equals: Unexpected font on test element expected 487 but got 532

  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • TIMEOUT [expected FAIL] subtest: sec-fetch-site - Cross-site, no attributes

      Test timed out

  • 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."

  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
    • TIMEOUT [expected FAIL] subtest: <dialog>-contained autofocus element gets focused when the dialog is shown

      Test timed out

  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/sandbox-top-navigation-cross-site.tentative.sub.window.html (#31754)
    • FAIL [expected TIMEOUT] subtest: A cross-site unsandboxed iframe navigation consumes user activation and disallows top-level navigation.

      promise_test: Unhandled rejection with value: object "TypeError: element.scrollIntoView is not a function"

  • OK /html/syntax/parsing/DOMContentLoaded-defer.html (#21550)
    • FAIL [expected PASS] subtest: The end: DOMContentLoaded and defer scripts

      assert_false: DOMContentLoaded should not have fired before executing a task queued from a defer script expected false got true

  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-delayed.html (#27659)
    • PASS [expected FAIL] subtest: async document.write in a module
  • TIMEOUT [expected OK] /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent

      Test timed out

    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent, even after history navigations by the parent
    • 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
Stable unexpected results (1)
  • FAIL [expected PASS] /css/css-tables/visibility-collapse-border-spacing-001.html

@github-actions GitHub Actions
Copy link

⚠️ Try run (#9452090865) failed.

@Loirooriol
Fix table track offsets when there is visibility: collapse
8954cd3 
Each non-collapsed track used to increase the offset by the subsequent
border spacing. Now they will take care of their preceding spacing
instead.

This way, if a cell spans two rows, and the second is collapsed, the
cell won't be forced to be at least as tall as the border spacing.
This matches Gecko and Blink (WebKit lacks `visibility: collapse`).

This makes visibility-collapse-border-spacing-001.html fail because we
generate outlines in a different way than Blink. Gecko also fails it
in a similar (but different) way.
@Loirooriol Loirooriol force-pushed the track-offsets-visibility-collapse branch from ab4301f to 8954cd3 Compare June 10, 2024 17:14
@Loirooriol Loirooriol added the T-linux-wpt-2020 Do a try run of the WPT label Jun 10, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Jun 10, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#9452688663) for Linux WPT

@servo-wpt-sync
Copy link
Collaborator

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

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (8)
  • TIMEOUT [expected OK] /css/css-transitions/disconnected-element-001.html (#32275)
    • TIMEOUT [expected PASS] subtest: Transitions are canceled when an element is re-parented

      Test timed out

    • NOTRUN [expected PASS] subtest: Transitions are canceled when an element is re-parented to the same node
  • OK /css/cssom-view/MediaQueryListEvent.html (#25275)
    • FAIL [expected PASS] subtest: argument of addListener

      assert_true: expected true got false

  • 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')
  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
  • TIMEOUT [expected OK] /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/remove-initial-about-blankness.window.html (#28684)
    • TIMEOUT [expected PASS] subtest: document.open() removes the initial about:blank-ness of the document

      Test timed out

  • TIMEOUT /resource-timing/test_resource_timing.html (#25720)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)

      assert_equals: expected 6359808 but got 6360064

  • 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 (10)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • FAIL [expected PASS] subtest: Matching font-style: 'oblique 0deg' should prefer 'oblique 15deg 20deg' over 'oblique 30deg 60deg'

      assert_equals: Unexpected font on test element expected 487 but got 532

  • OK /custom-elements/form-associated/ElementInternals-setFormValue.html (#29174)
    • PASS [expected FAIL] subtest: Null value should submit nothing
    • PASS [expected FAIL] subtest: Newline normalization - \n in value (formdata)
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • TIMEOUT [expected FAIL] subtest: sec-fetch-site - Cross-site, no attributes

      Test timed out

  • 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 [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
  • TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • FAIL [expected TIMEOUT] subtest: Element with tabindex should support autofocus

      assert_equals: expected "SPAN" but got "BODY"

    • TIMEOUT [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus

      Test timed out

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/sandbox-top-navigation-cross-site.tentative.sub.window.html (#31754)
    • FAIL [expected TIMEOUT] subtest: A cross-site unsandboxed iframe navigation consumes user activation and disallows top-level navigation.

      promise_test: Unhandled rejection with value: object "TypeError: element.scrollIntoView is not a function"

  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: Basic File test (formdata event)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: backslash in value (normal form)
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-delayed.html (#27659)
    • PASS [expected FAIL] subtest: async document.write in a module

@github-actions GitHub Actions
Copy link

✨ Try run (#9452688663) succeeded.

@mrobinson mrobinson added this pull request to the merge queue Jun 11, 2024
Merged via the queue into servo:main with commit b4e41d8 Jun 11, 2024
37 checks passed
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
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Collapsed table tracks are wrong with spanning cells
3 participants
@Loirooriol @servo-wpt-sync @mrobinson