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.

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

Add value argument to URLSearchParams's has() and delete() #29726

Merged
merged 1 commit into from
May 11, 2023

Conversation

paricbat
Copy link
Contributor

@paricbat paricbat commented May 8, 2023

My first attempt at contributing Rust code!
Add the value argument to has() and done() as in the spec.


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

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 work. One small nit...

Comment on lines 146 to 148
list.iter().any(|&(ref k, ref v)| {
if let Some(value) = &value {
k == &name.0 && v == &value.0
} else {
k == &name.0
}
})
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 this could be rewritten as a match statement:

           match &value {
               Some(value)  => k == &name.0 && v == &value.0
               None =>  k == &name.0
            }

// Step 1.
self.list.borrow_mut().retain(|&(ref k, _)| k != &name.0);
self.list.borrow_mut().retain(|&(ref k, ref v)| {
Copy link
Member

Choose a reason for hiding this comment

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

See my comment below...

@mrobinson
Copy link
Member

@bors-servo try=wpt-2020

@bors-servo
Copy link
Contributor

⌛ Trying commit f46a05a with merge 3356a64...

bors-servo added a commit that referenced this pull request May 9, 2023
Add value argument to URLSearchParams's has() and delete()

<!-- Please describe your changes on the following line: -->
My first attempt at contributing Rust code!
Add the value argument to `has()` and `done()` as in [the spec](https://url.spec.whatwg.org/#dom-urlsearchparams-delete).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix  #29725 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@mrobinson
Copy link
Member

I suspect you will also need to update results for Layout 2020.

@github-actions
Copy link

github-actions bot commented May 9, 2023

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

Stable unexpected results that are known to be intermittent (1)
  • PASS [expected TIMEOUT] /css/css-color/animation/opacity-animation-ending-correctly-001.html (#29215)

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
State: approved= try=True

@paricbat
Copy link
Contributor Author

paricbat commented May 9, 2023

Okay, it should be fixed-up now

This commit should fix servo#29725.

Signed-off-by: Veronika Bušů <paricbat@email.cz>
@mrobinson
Copy link
Member

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit e40ec79 has been approved by mrobinson

@bors-servo
Copy link
Contributor

⌛ Testing commit e40ec79 with merge 425b0fe...

@github-actions
Copy link

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

Flaky unexpected result (17)
  • 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
  • OK /css/CSS2/linebox/inline-negative-margin-001.html (#23862)
    • FAIL [expected PASS] subtest: [data-expected-height] 1 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">1234 </span></inline-block>
      height expected 10 but got 20
    • FAIL [expected PASS] subtest: [data-expected-height] 2 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">123 </span></inline-block>
      height expected 10 but got 20
    • PASS [expected FAIL] subtest: [data-expected-height] 4
  • FAIL [expected PASS] /css/compositing/root-element-opacity-change.html (#26366)
  • OK /css/css-flexbox/text-as-flexitem-size-001.html (#28726)
    • FAIL [expected PASS] subtest: .flexbox > div 1 assert_equals:
      <div data-expected-width="70" data-expected-height="35">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 2 assert_equals:
      <div data-expected-width="50" data-expected-height="45">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 50 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 5 assert_equals:
      <div style="height: 30px" data-expected-width="70" data-expected-height="30">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 8 assert_equals:
      <div style="min-height: 40px" data-expected-width="70" data-expected-height="40">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 11 assert_equals:
      <div style="max-height: 20px" data-expected-width="70" data-expected-height="20">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
  • FAIL [expected PASS] /css/css-text/line-breaking/segment-break-transformation-unremovable-1.html (#28758)
  • 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/navigation-unload-cross-origin.sub.window.html (#29056)
    • PASS [expected FAIL] subtest: Cross-origin navigation started from unload handler must be ignored
  • 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
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/skip-another-top-level-browsing-context.html (#24161)
    • TIMEOUT [expected PASS] subtest: Autofocus elements queued in another top-level browsing context's documents should be skipped. Test timed out
  • 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
  • OK [expected ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • ERROR /resource-timing/content-type-parsing.html (#29131)
    • FAIL [expected TIMEOUT] subtest: mime-type 16 : text/html;charset=�gbk assert_equals: expected (string) "text/html" but got (undefined) undefined
    • TIMEOUT [expected NOTRUN] subtest: mime-type 17 : text/html;charset= gbk Test timed out
  • TIMEOUT /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that iframe navigations are not observable by the parent, even after history navigations by the parent Test timed out
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent
    • 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 TIMEOUT] subtest: Test that iframe refreshes are not observable by the parent
  • CRASH [expected TIMEOUT] /webmessaging/broadcastchannel/cross-partition.https.tentative.html (#29058)
  • TIMEOUT [expected OK] /webmessaging/with-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank Test timed out
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out
Stable unexpected results that are known to be intermittent (13)
  • OK [expected TIMEOUT] /fetch/api/basic/keepalive.any.html (#29536)
  • OK [expected TIMEOUT] /fetch/local-network-access/iframe.tentative.https.window.html (#29605)
    • PASS [expected TIMEOUT] subtest: local to local, grandparent navigates: no preflight required.
    • FAIL [expected TIMEOUT] subtest: public to local, grandparent navigates: failure. timeout adding grandchild
    • FAIL [expected TIMEOUT] subtest: public to local, grandparent navigates: success. timeout adding grandchild
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • PASS [expected TIMEOUT] subtest: background-image sec-fetch-dest - Not sent to non-trustworthy same-site destination
    • FAIL [expected PASS] subtest: border-image sec-fetch-user - Not sent to non-trustworthy cross-site destination assert_unreached: Reached unreachable code
    • FAIL [expected PASS] subtest: border-image sec-fetch-site - HTTPS downgrade (header not sent) assert_unreached: Reached unreachable code
  • TIMEOUT /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • TIMEOUT [expected FAIL] subtest: Navigating to a different document with link click Test timed out
    • NOTRUN [expected TIMEOUT] subtest: Navigating to a different document with form submission
  • TIMEOUT /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-no-beforeunload.window.html (#29055)
    • PASS [expected TIMEOUT] subtest: Navigating an opened window via location.href to a javascript: URL must not fire beforeunload: undefined completion
    • FAIL [expected NOTRUN] subtest: Navigating an opened window with an iframe via location.href to a javascript: URL must not fire beforeunload on the iframe: undefined completion promise_test: Unhandled rejection with value: object "TypeError: w.frames[0] is undefined"
    • PASS [expected NOTRUN] subtest: Navigating an iframe via location.href to a javascript: URL must not fire beforeunload: string completion
    • PASS [expected NOTRUN] subtest: Navigating an iframe via src="" to a javascript: URL after insertion must not fire beforeunload: string completion
    • TIMEOUT [expected NOTRUN] subtest: Navigating an opened window via location.href to a javascript: URL must not fire beforeunload: string completion Test timed out
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin-fragment.html (#20768)
    • PASS [expected FAIL] subtest: Tests that a fragment navigation in the unload handler will not block the initial navigation
  • OK /html/browsers/history/the-history-interface/traverse_the_history_1.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals from the same task assert_array_equals: Pages opened during history navigation expected property 1 to be 2 but got 1 (expected array [4, 2] got [4, 1])
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/update-the-rendering.html (#24145)
    • FAIL [expected TIMEOUT] subtest: "Flush autofocus candidates" should be happen before a scroll event and animation frame callbacks w.focus is not a function
  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • FAIL [expected PASS] subtest: multipart/form-data: Basic test (formdata event) assert_equals: expected "\r\nContent-Disposition: form-data; name="basic"\r\n\r\ntest\r\n--\r\n" but got ""
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • FAIL [expected PASS] subtest: text/plain: Basic File test (formdata event) assert_equals: expected "basic=file-test.txt\r\n" but got ""
  • OK [expected TIMEOUT] /html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html (#23205)
    • PASS [expected NOTRUN] subtest: Check that rel=noopener with target=_self does a normal load
    • FAIL [expected NOTRUN] subtest: Check that rel=noopener with target=_top does a normal load this.openedWindow.findLink is not a function
  • OK [expected TIMEOUT] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
  • 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

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
Approved by: mrobinson
Pushing 425b0fe to master...

@bors-servo bors-servo merged commit 425b0fe into servo:master May 11, 2023
10 checks passed
@paricbat paricbat deleted the urlsearchparams-has-delete branch July 10, 2023 16:52
@paricbat paricbat restored the urlsearchparams-has-delete branch July 10, 2023 16:52
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.

Add value argument to URLSearchParams's has() and delete()
3 participants