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

Implemented HTMLFormElement.relList #27255

Merged
merged 1 commit into from Jul 14, 2020
Merged

Implemented HTMLFormElement.relList #27255

merged 1 commit into from Jul 14, 2020

Conversation

avr1254
Copy link
Contributor

@avr1254 avr1254 commented Jul 13, 2020

Updated the tests to reflect addition of rel and relList for HTMLFormElement, as well as porting those code snippets from HTMLAnchorElement.


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

@highfive
Copy link

highfive commented Jul 13, 2020

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @nox (or someone else) soon.

@highfive
Copy link

highfive commented Jul 13, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLFormElement.webidl
  • @KiChjang: components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLFormElement.webidl

@jdm
Copy link
Member

jdm commented Jul 13, 2020

@bors-servo try=wpt

bors-servo added a commit that referenced this issue Jul 13, 2020
Resolved #27252:   Implemented HTMLFormElement.relList

<!-- Please describe your changes on the following line: -->
Updated the tests to reflect addition of rel and relList for HTMLFormElement, as well as porting those code snippets from HTMLAnchorElement.

---
<!-- 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 #27252  (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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 13, 2020

Trying commit a31e719 with merge 7d37d5b...

@jdm jdm assigned jdm and unassigned nox Jul 13, 2020
@bors-servo
Copy link
Contributor

bors-servo commented Jul 13, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jul 13, 2020

A couple other test results need to be updated:

  ▶ Unexpected subtest result in /WebIDL/ecmascript-binding/put-forwards.html:
  └ PASS [expected FAIL] Setting form.relList to noreferrer is reflected in rel
  ▶ Unexpected subtest result in /html/dom/idlharness.https.html?include=HTML.*:
  └ PASS [expected FAIL] HTMLFormElement interface: document.createElement("form") must inherit property "rel" with the proper type
  ▶ Unexpected subtest result in /html/dom/idlharness.https.html?include=HTML.*:
  └ PASS [expected FAIL] HTMLFormElement interface: document.createElement("form") must inherit property "relList" with the proper type

That's tests/wpt/metadata/WebIDL/emcascript-binding/put-forwards.html.ini and tests/wpt/metadata/html/dom/idlharness.https.html.ini.

@avr1254
Copy link
Contributor Author

avr1254 commented Jul 13, 2020

Thanks! Updating now.

@avr1254
Copy link
Contributor Author

avr1254 commented Jul 14, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

@avr1254: 🔑 Insufficient privileges: not in try users

@avr1254
Copy link
Contributor Author

avr1254 commented Jul 14, 2020

I don't have a clue what went wrong this time. Maybe it's my configuration?

@avr1254 avr1254 changed the title Resolved #27252: Implemented HTMLFormElement.relList Implemented HTMLFormElement.relList Jul 14, 2020
@CYBAI
Copy link
Member

CYBAI commented Jul 14, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

Trying commit de1a9bb with merge ef7e790...

bors-servo added a commit that referenced this issue Jul 14, 2020
Implemented HTMLFormElement.relList

<!-- Please describe your changes on the following line: -->
Updated the tests to reflect addition of rel and relList for HTMLFormElement, as well as porting those code snippets from HTMLAnchorElement.

---
<!-- 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 #27252  (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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

☀️ Test successful - status-taskcluster
State: approved= try=True

jdm
jdm approved these changes Jul 14, 2020
Copy link
Member

@jdm jdm left a comment

This looks good! Could you squash all the commits into one?

@@ -1189,7 +1208,6 @@ impl HTMLFormElement {
// Step 9
Some(form_data.datums())
}

Copy link
Member

@jdm jdm Jul 14, 2020

Choose a reason for hiding this comment

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

nit: could you add this newline back?

@@ -1,4 +1,3 @@
[put-forwards.html]
type: testharness
[Setting form.relList to noreferrer is reflected in rel]
expected: FAIL
Copy link
Member

@jdm jdm Jul 14, 2020

Choose a reason for hiding this comment

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

This file can be removed now that it doesn't contain any non-pass results.

@avr1254
Copy link
Contributor Author

avr1254 commented Jul 14, 2020

I'm having a bit of trouble rebasing, since I accidentally updated my local repository with commits on the master here. Any advice?

@jdm
Copy link
Member

jdm commented Jul 14, 2020

@avr1254 If you can find the last commit you made to the branch in git reflog, it might be easiest to do a hard reset to that revision and try the squash again.

@avr1254
Copy link
Contributor Author

avr1254 commented Jul 14, 2020

Yeah, I think this looks good now. It's only my commits.

@jdm
Copy link
Member

jdm commented Jul 14, 2020

@bors-servo r+
Thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

📌 Commit aff3434 has been approved by jdm

…ement

Updated tests to reflect rel and relList in HTMLFormElement

Added AttrValue as style

Added attr

Updated outstanding test cases

Fixed formatting. Hopefully this time works

Implemented HTMLFormElement.relList
@jdm
Copy link
Member

jdm commented Jul 14, 2020

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

📌 Commit 00f69dd has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

Testing commit 00f69dd with merge c8d0548...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing c8d0548 to master...

@bors-servo bors-servo merged commit c8d0548 into servo:master Jul 14, 2020
2 checks passed
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.

6 participants