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

implement HTMLOutputElement.value #25412

Merged
merged 1 commit into from Jan 7, 2020
Merged

implement HTMLOutputElement.value #25412

merged 1 commit into from Jan 7, 2020

Conversation

@pshaughn
Copy link
Member

pshaughn commented Jan 1, 2020

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25002 (GitHub issue number if applicable)
  • There are tests for these changes
@highfive
Copy link

highfive commented Jan 1, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmloutputelement.rs, components/script/dom/node.rs, components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLOutputElement.webidl
  • @KiChjang: components/script/dom/htmloutputelement.rs, components/script/dom/node.rs, components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLOutputElement.webidl
@pshaughn
Copy link
Member Author

pshaughn commented Jan 1, 2020

I am certain this is passing more tests than I've already fixed metadata for.
@bors-servo try=wpt

bors-servo added a commit that referenced this pull request Jan 1, 2020
Now passing output tests as well as anyone

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 1, 2020

Trying commit 13f40df with merge de5c565...

@pshaughn pshaughn changed the title Now passing output tests as well as anyone implement HTMLOutputElement.value Jan 1, 2020
@bors-servo
Copy link
Contributor

bors-servo commented Jan 1, 2020

💔 Test failed - status-taskcluster

@pshaughn pshaughn force-pushed the pshaughn:outputvalue branch from 13f40df to fdef9c3 Jan 1, 2020
@highfive highfive removed the S-tests-failed label Jan 1, 2020
@pshaughn
Copy link
Member Author

pshaughn commented Jan 1, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jan 1, 2020

Trying commit fdef9c3 with merge a47aa4f...

bors-servo added a commit that referenced this pull request Jan 1, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 1, 2020

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

components/script/dom/node.rs Show resolved Hide resolved
@@ -2017,6 +2017,16 @@ impl Node {
parent.owner_doc().remove_script_and_layout_blocker();
}

// https://dom.spec.whatwg.org/multipage/#string-replace-all
pub fn string_replace_all(string: DOMString, parent: &Node) {

This comment has been minimized.

@Manishearth

Manishearth Jan 2, 2020

Member

Should this be a method on node?

This comment has been minimized.

@pshaughn

pshaughn Jan 2, 2020

Author Member

I went for consistency with replace_all:

// https://dom.spec.whatwg.org/#concept-node-replace-all
pub fn replace_all(node: Option<&Node>, parent: &Node) {

This comment has been minimized.

@Manishearth

Manishearth Jan 3, 2020

Member

Oh, wait, I thought it wasn't on Node, and then was suggesting that it should be. I didn't notice that it was :)

@Manishearth
Copy link
Member

Manishearth commented Jan 3, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 3, 2020

📌 Commit e348c33 has been approved by Manishearth

bors-servo added a commit that referenced this pull request Jan 3, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 3, 2020

Testing commit e348c33 with merge af53ab4...

@bors-servo
Copy link
Contributor

bors-servo commented Jan 3, 2020

💔 Test failed - status-taskcluster

@pshaughn
Copy link
Member Author

pshaughn commented Jan 3, 2020

Oh, this was just that Mac filter-intermittents python thing...
@bors-servo try

@bors-servo
Copy link
Contributor

bors-servo commented Jan 3, 2020

Trying commit e348c33 with merge 858d2ae...

bors-servo added a commit that referenced this pull request Jan 3, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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. -->
@jdm
Copy link
Member

jdm commented Jan 3, 2020

No need to run a full try. We can retry the merge instead.
@bors-servo try- retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 3, 2020

Testing commit e348c33 with merge d50e623...

bors-servo added a commit that referenced this pull request Jan 6, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 6, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 7, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2020

Testing commit a322c60 with merge a2f26d0...

bors-servo added a commit that referenced this pull request Jan 7, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 7, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 7, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2020

Testing commit a322c60 with merge 0e394b5...

bors-servo added a commit that referenced this pull request Jan 7, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 7, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 7, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2020

Testing commit a322c60 with merge 709e069...

bors-servo added a commit that referenced this pull request Jan 7, 2020
implement HTMLOutputElement.value

HTMLOutputElement now works. I believe layout as well as script is working for it, since it works by putting strings into a child Text node and lays itself out exactly like a span, not requiring any special case rendering like textareas or input elements.

Remaining failures are one mutation test case that everyone else is also failing and probably represents a test/spec mismatch, and all the validation-related tests since we don't have any form-validation hooks.

---
<!-- 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 #25002 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- 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 Jan 7, 2020

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

@bors-servo bors-servo merged commit a322c60 into servo:master Jan 7, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.