Skip to content

layout: Use a total order comparator when distributing extra space to rowspans - #46841

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:table-total-order-sort
Jul 28, 2026
Merged

layout: Use a total order comparator when distributing extra space to rowspans#46841
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:table-total-order-sort

Conversation

@mrobinson

Copy link
Copy Markdown
Member

Extra space is distributed to rowspans in an order determined by the
specification. The sorting that was used to determine this order wasn't
total which meant that you could have a situation where:

  • rowspan_a == rowspan_b
  • rowspan_a > rowspan_c
  • rowspan_c > rowspan_b

This change makes the sorting total by properly ordering ranges even
when one side of their ranges overlap. The test included here
unfortunately has to be a bit noisy because the standard library only
detects a non-total ordering under a specific set and distribution of
elements.

Testing: This change includes a WPT crashtest.
Fixes: #46806.

… rowspans

Extra space is distributed to rowspans in an order determined by the
specification. The sorting that was used to determine this order wasn't
total which meant that you could have a situation where:

 - rowspan_a == rowspan_b
 - rowspan_a > rowspan_c
 - rowspan_c > rowspan_b

This change makes the sorting total by properly ordering ranges even
when one side of their ranges overlap. The test included here
unfortunately has to be a bit noisy because the standard library only
detects a non-total ordering under a specific set and distribution of
elements.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@mrobinson
mrobinson requested a review from Loirooriol as a code owner July 28, 2026 17:35
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 28, 2026
@servo-wpt-sync

Copy link
Copy Markdown
Collaborator

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

@mrobinson mrobinson added the T-linux-wpt Do a try run of the WPT label Jul 28, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

Test results for linux-wpt from try job (#30384655974):

Flaky unexpected result (32)
  • CRASH [expected OK] /_mozilla/mozilla/focus-scroll-when-visible.html
  • TIMEOUT [expected OK] /_webgl/conformance/extensions/oes-texture-float-linear.html
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/textures/misc/texture-video-transparent.html
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance2/sync/sync-webgl-specific.html
  • CRASH [expected OK] /css/compositing/canvas-composite-modes.html
  • FAIL [expected PASS] /css/css-sizing/dynamic-available-size-iframe.html (#45763)
  • FAIL [expected PASS] /css/css-text-decor/text-underline-position-from-font-variable.html (#45615)
  • TIMEOUT [expected OK] /fetch/content-encoding/br/big-br-body.https.any.html
    • TIMEOUT [expected PASS] subtest: large br data should be decompressed successfully

      Test timed out
      

    • NOTRUN [expected FAIL] subtest: large br data should be decompressed successfully with byte stream
  • TIMEOUT [expected OK] /fetch/content-encoding/br/big-br-body.https.any.worker.html
    • TIMEOUT [expected PASS] subtest: large br data should be decompressed successfully

      Test timed out
      

    • NOTRUN [expected FAIL] subtest: large br data should be decompressed successfully with byte stream
  • TIMEOUT [expected OK] /fetch/content-encoding/gzip/big-gzip-body.https.any.worker.html
    • TIMEOUT [expected PASS] subtest: large gzip data should be decompressed successfully

      Test timed out
      

    • NOTRUN [expected FAIL] subtest: large gzip data should be decompressed successfully with byte stream
  • TIMEOUT /fetch/metadata/generated/css-images.https.sub.tentative.html (#42229)
    • FAIL [expected PASS] subtest: content sec-fetch-site - Cross-Site -&gt; Cross-Site

      assert_unreached: Reached unreachable code
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/005.html (#27062)
    • PASS [expected FAIL] subtest: Link with onclick navigation and href navigation
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html (#44099)
    • FAIL [expected TIMEOUT] subtest: Replace before load, triggered by submitButton.click()

      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)&amp;code=%0A%20%20%20%20const%20form%20%3D%20document.createElement(%22form%22)%3B%0A%20%20%20%20form.action%20%3D%20%22%2Fcommon%2Fblank.html%22%3B%0A%0A%20%20%20%20const%20input%20%3D%20document.createElement(%22input%22)%3B%0A%20%20%20%20input.type%20%3D%20%22hidden%22%3B%0A%20%20%20%20input.name%20%3D%20%22thereplacement%22%3B%0A%20%20%20%20form.append(input)%3B%0A%0A%20%20%20%20const%20button%20%3D%20document.createElement(%22button%22)%3B%0A%20%20%20%20button.type%20%3D%20%22submit%22%3B%0A%20%20%20%20form.append(button)%3B%0A%0A%20%20%20%20document.currentScript.before(form)%3B%0A%20%20%20%20button.click()%3B%0A%20%20"
      

  • 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/windows/embedded-opener-remove-frame.html (#23867)
    • PASS [expected FAIL] subtest: opener of discarded auxiliary browsing context
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • TIMEOUT [expected FAIL] subtest: Host element with delegatesFocus should support autofocus

      Test timed out
      

    • NOTRUN [expected FAIL] subtest: Host element with delegatesFocus including no focusable descendants should be skipped
    • NOTRUN [expected FAIL] subtest: Area element should support autofocus
  • TIMEOUT /html/semantics/embedded-content/media-elements/autoplay-default-permissions-policy.https.sub.html (#46805)
    • PASS [expected TIMEOUT] subtest: Default "autoplay" feature policy ["self"] allows same-origin iframes.
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
    • TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup

      Test timed out
      

  • OK /html/semantics/forms/form-submission-0/jsurl-form-submit.tentative.html (#36489)
    • PASS [expected FAIL] subtest: Verifies that form submissions scheduled inside javascript: urls take precedence over the javascript: url's return value.
  • TIMEOUT [expected OK] /html/semantics/links/links-created-by-a-and-area-elements/rel-opener-prevents-browsing-context-group-change.tentative.html
    • TIMEOUT [expected PASS] subtest: rel=opener prevents browsing context group change

      Test timed out
      

    • NOTRUN [expected PASS] subtest: opener window feature prevents browsing context group change
  • TIMEOUT [expected OK] /html/syntax/parsing/html5lib_write_single.html?file=comments01
    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR --&gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR --!&gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR --! &gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR --!\n&gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR -- &gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR -- &lt;QUX&gt; -- MUX --&gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR -- &lt;QUX&gt; -- MUX --!&gt;BAZ

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: FOO&lt;!-- BAR -- &lt;QUX&gt; -- MUX -- &gt;BAZ

      Test timed out
      

    • NOTRUN [expected PASS] subtest: FOO&lt;!----&gt;BAZ
    • NOTRUN [expected PASS] subtest: FOO&lt;!---&gt;BAZ
    • And 6 more unexpected results...
  • TIMEOUT [expected OK] /html/user-activation/navigation-state-reset-sameorigin.html (#45151)
    • TIMEOUT [expected FAIL] subtest: Post-navigation state reset.

      Test timed out
      

  • OK /html/webappapis/dynamic-markup-insertion/document-write/iframe_005.html (#43671)
    • PASS [expected FAIL] subtest: document.write external script into iframe write back into parent
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-dynamic-import.html
    • FAIL [expected PASS] subtest: document.write in an imported module

      assert_true: onload must be called expected true got false
      

  • ERROR [expected OK] /intersection-observer/v2/animated-opacity.html (#40827)
  • CRASH [expected OK] /svg/animations/animate-calcMode-spline-from-by.html
  • CRASH [expected TIMEOUT] /svg/interact/scripted/defer-02.html
  • CRASH [expected ERROR] /svg/painting/parsing/paint-order-invalid.svg
  • CRASH [expected OK] /trusted-types/require-trusted-types-for.html
  • OK /upgrade-insecure-requests/gen/sharedworker-module-data.meta/upgrade/xhr.https.html
    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to cross-http-downgrade origin and downgrade redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to cross-http-downgrade origin and no-redirect redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to cross-https origin and downgrade redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to same-http-downgrade origin and downgrade redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to same-http-downgrade origin and no-redirect redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

    • FAIL [expected PASS] subtest: Upgrade-Insecure-Requests: Expects allowed for xhr to same-https origin and downgrade redirection from https context.

      assert_equals: The resource request should be 'allowed'. expected "allowed" but got "blocked"
      

  • OK /visual-viewport/resize-event-order.html (#41981)
    • PASS [expected FAIL] subtest: Popup: DOMWindow resize fired before VisualViewport.
Stable unexpected results that are known to be intermittent (16)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • TIMEOUT [expected PASS] subtest: Fetching a blob URL immediately before revoking it works in &lt;script&gt; tags.

      Test timed out
      

  • OK /beacon/beacon-basic.https.window.html (#41723)
    • PASS [expected FAIL] subtest: Payload size restriction should be accumulated: type = arraybuffer
    • PASS [expected FAIL] subtest: Payload size restriction should be accumulated: type = blob
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • FAIL [expected PASS] subtest: Matching font-weight: '501' should prefer '500' over '450 460'

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

  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • TIMEOUT [expected PASS] subtest: background-image sec-fetch-site - Not sent to non-trustworthy same-origin destination

      Test timed out
      

    • FAIL [expected PASS] subtest: content sec-fetch-storage-access - Not sent to non-trustworthy cross-site destination

      assert_unreached: 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."
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/refresh/same-document-refresh.html (#34597)
    • PASS [expected FAIL] subtest: Same-Document Referrer from Refresh
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit.html (#44028)
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
    • FAIL [expected TIMEOUT] subtest: &lt;dialog&gt;-contained autofocus element gets focused when the dialog is shown

      promise_test: Unhandled rejection with value: object "TypeError: can't access property "show", w.document.querySelector(...) is null"
      

  • OK [expected TIMEOUT] /html/syntax/parsing/html5lib_write.html?file=processing-instructions (#46205)
  • TIMEOUT [expected OK] /html/syntax/parsing/html5lib_write_single.html?file=processing-instructions (#46206)
    • TIMEOUT [expected PASS] subtest: &lt;body&gt;&lt;?xML&gt;

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: &lt;body&gt;&lt;?xml-STYLesheet&gt;

      Test timed out
      

    • TIMEOUT [expected PASS] subtest: &lt;body&gt;&lt;?1st-place&gt;

      Test timed out
      

  • OK /pointerevents/pointerevent_change-touch-action-onpointerdown_touch.html (#46781)
    • FAIL [expected PASS] subtest: scroll was received while shouldn't

      assert_true: scroll received while shouldn't expected true got false
      

  • OK /resource-timing/buffer-full-add-then-clear.html (#40819)
    • FAIL [expected PASS] subtest: Test that if the buffer is cleared after entries were added to the secondary buffer, those entries make it into the primary one

      assert_equals: Number of entries does not match the expected value. expected 3 but got 0
      

  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?01-05 (#38975)
    • PASS [expected TIMEOUT] subtest: Navigate a window via anchor with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via anchor with javascript:-urls in enforcing mode.
  • TIMEOUT [expected OK] /trusted-types/trusted-types-navigation.html?16-20 (#44835)
    • TIMEOUT [expected PASS] subtest: Navigate a frame via area with javascript:-urls w/ default policy in report-only mode.

      Test timed out
      

  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?26-30 (#38807)
    • PASS [expected TIMEOUT] subtest: Navigate a window via form-submission with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in enforcing mode.
  • ERROR [expected OK] /webxr/render_state_update.https.html (#27535)

@github-actions

Copy link
Copy Markdown

⚠️ Try run (#30384655974) failed!

@mrobinson
mrobinson added this pull request to the merge queue Jul 28, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 28, 2026
Merged via the queue into servo:main with commit d60b660 Jul 28, 2026
74 of 76 checks passed
@mrobinson
mrobinson deleted the table-total-order-sort branch July 28, 2026 19:28
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 28, 2026
/// Returns true if other is a proper subset of [`self`].
fn fully_encloses(&self, other: &RowspanToDistribute) -> bool {
other.coordinates.y > self.coordinates.y && other.range().end < self.range().end
self.range() != other.range() &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

self.range() != other.range() is already ensured by the caller.

Seems a bit weird to use non-strict inequalities, but then require the ranges to be different.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The main goal here is to handle the case where only end of the range overlaps (e.g. [0..3] and [0..5]). Maybe this should be renamed to is_proper_subset and the caller's code reordered?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash on https://it.wikipedia.org/wiki/Giovanni_di_Bicci_de'_Medici "user-provided comparison function does not correctly implement a total order"

5 participants