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

Implement additional logic in DOMString::set_best_representation_of_the_floating_point_number #32272

Merged
merged 1 commit into from May 13, 2024
Merged

Implement additional logic in DOMString::set_best_representation_of_the_floating_point_number #32272

merged 1 commit into from May 13, 2024

Conversation

shanehandley
Copy link
Contributor

@shanehandley shanehandley commented May 12, 2024 •
edited

This corrects some failing tests related to "-0.0" values for the meter element, as well as updates input number and range elements - these were not failing similar tests, it appears that the default fallback was accounting for -0.0 values.

There are also some fixes to the progress element where it was mishandling negative values for max.

See #32269 for additional context.

  • There are tests for these changes OR
  • These changes do not require tests because ___

@shanehandley
fix: Implement additional logic in DOMString::set_best_representation…
3e03df1 
…_of_the_floating_point_number in order to correct some failing tests related to -0 values.
sagudev
sagudev reviewed May 12, 2024
View reviewed changes
tests/wpt/meta/html/dom/reflection-forms.html.ini

[progress.max: IDL set to 0]
expected: FAIL

[progress.max: IDL set to 1e-10]
Copy link
Member

Choose a reason for hiding this comment

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

I think there are even more tests that we could fix with this (look for IDL set to x).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you - I did have a longer look into those. I think there's an edge case in our implementation of is_valid_floating_point_number_string that fails for 1.e2. I'm pretty sure this one should be valid, but our regex enforces that a digit appears after the decimal point.

Edit - I was wrong, it's fine 👍

@mrobinson mrobinson 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 May 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 May 13, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#9058770016) for Linux WPT

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (9)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '501' should prefer '503 520' over '500'
    • PASS [expected FAIL] subtest: Matching font-stretch: '110%' should prefer '105%' over '100%'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'oblique 20deg 30deg' over 'oblique -50deg -20deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 21deg' should prefer 'oblique 20deg' over 'oblique 10deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 10deg' should prefer 'oblique 15deg 20deg' over 'oblique 30deg 60deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 0deg' should prefer 'oblique -50deg -20deg' over 'oblique -40deg -30deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique -20deg -15deg' over 'oblique -60deg -30deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique -50deg -40deg' over 'italic'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique 0deg 10deg' over 'oblique 40deg 50deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -20deg' should prefer 'oblique -20deg' over 'oblique -60deg -40deg'
    • And 3 more unexpected results...
  • OK /css/css-values/cap-invalidation.html
    • 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] /fetch/api/redirect/redirect-keepalive.https.any.html (#32153)
    • 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
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/sandbox-top-navigation-same-site.tentative.sub.window.html (#32117)
    • TIMEOUT [expected FAIL] subtest: A same-site unsandboxed iframe navigation does not consume user activation and allows top-level navigation.

      Test timed out

  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: backslash in value (formdata event)
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry.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

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

      assert_equals: expected 24186624 but got 24186880

  • TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.tentative.html (#29053)
    • TIMEOUT [expected PASS] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe

      Test timed out

Stable unexpected results that are known to be intermittent (15)
  • FAIL [expected PASS] /_mozilla/gfx-rs-gecko/descriptor-ranges.html (#23258)
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • FAIL [expected PASS] subtest: listeners are called correct number of times

      assert_equals: expected 5 but got 4

  • TIMEOUT /css/cssom-view/MediaQueryList-extends-EventTarget.html (#25269)
    • TIMEOUT [expected PASS] subtest: onchange removes listener

      Test timed out

    • NOTRUN [expected PASS] subtest: listeners for "change" type are called
    • NOTRUN [expected PASS] subtest: listeners with different type are not called
    • NOTRUN [expected TIMEOUT] subtest: addEventListener "once" option is respected
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • FAIL [expected TIMEOUT] subtest: sec-fetch-site - Cross-site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • FAIL [expected NOTRUN] subtest: sec-fetch-site - Same site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • TIMEOUT [expected NOTRUN] subtest: sec-fetch-site - Same-Origin -> Cross-Site -> Same-Origin redirect, 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."

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

      assert_equals: expected "?pass" but got "?fail"

  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.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)
    • 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 CRASH] /html/dom/documents/dom-tree-accessors/Document.currentScript.html (#22423)
  • TIMEOUT [expected OK] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
  • ERROR [expected OK] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-iframes-without-attributes.tentative.window.html (#32213)
    • FAIL [expected TIMEOUT] subtest: RemoteContextHelper navigation using BFCache

      promise_test: Unhandled rejection with value: object "Error: AbortController is not defined"

@github-actions GitHub Actions
Copy link

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

Flaky unexpected result (11)
  • CRASH [expected PASS] /_webgl/conformance2/textures/misc/compressed-tex-from-pbo-crash.html
  • TIMEOUT /fetch/fetch-later/new-window.tentative.https.window.html (#32036)
    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='0'] can trigger fetchLater.

      Test timed out

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='1'] can trigger fetchLater.

      Test timed out

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='2'] can trigger fetchLater.

      Test timed out

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='0'] can trigger fetchLater.

      Test timed out

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='1'] can trigger fetchLater.

      Test timed out

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='2'] can trigger fetchLater.

      Test timed out

  • 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
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html (#28765)
    • TIMEOUT [expected FAIL] subtest: Replace before load, triggered by submitButton.click()

      Test timed out

  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
    • PASS [expected FAIL] subtest: first argument: absolute url
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • TIMEOUT [expected OK] /html/semantics/document-metadata/the-base-element/base_target_does_not_affect_location_assignment.html
    • TIMEOUT [expected PASS] subtest: base_target_does_not_affect_location_assignment

      Test timed out

  • CRASH [expected PASS] /html/semantics/embedded-content/the-img-element/image-loading-lazy-subframe-detached-crash.html (#31572)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: 0x00 in name (normal form)
  • TIMEOUT [expected OK] /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/remove-initial-about-blankness.window.html (#28684)
    • TIMEOUT [expected FAIL] subtest: Double-check: without document.open(), Window reuse indeed happens

      Test timed out

  • 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 that are known to be intermittent (11)
  • FAIL [expected PASS] /_mozilla/gfx-rs-gecko/descriptor-ranges.html (#23258)
  • 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/replace-before-load/a-click.html (#28697)
    • PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
  • 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])

  • TIMEOUT [expected CRASH] /html/dom/documents/dom-tree-accessors/Document.currentScript.html (#22423)
  • OK /html/interaction/focus/the-autofocus-attribute/document-with-fragment-valid.html (#28259)
    • PASS [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with URL fragments should be skipped.
  • TIMEOUT /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
    • TIMEOUT [expected FAIL] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      Test timed out

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • TIMEOUT [expected FAIL] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      Test timed out

  • OK /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
    • 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-2.html (#22154)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)

@github-actions GitHub Actions
Copy link

✨ Try run (#9058770016) succeeded.

@mrobinson mrobinson added this pull request to the merge queue May 13, 2024
Merged via the queue into servo:main with commit 8eeb888 May 13, 2024
61 checks passed
@shanehandley shanehandley deleted the best_repr_foats branch May 13, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagudev sagudev sagudev left review comments

@mrobinson mrobinson mrobinson approved these changes

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

Successfully merging this pull request may close these issues.

parse_floating_point_number does not conform to spec
3 participants
@shanehandley @mrobinson @sagudev