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 XMLHttpRequest.send(Document) #9490

Closed
Ms2ger opened this issue Feb 1, 2016 · 22 comments
Closed

Implement XMLHttpRequest.send(Document) #9490

Ms2ger opened this issue Feb 1, 2016 · 22 comments

Comments

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Feb 1, 2016

Blocks #9489.

bors-servo added a commit that referenced this issue Feb 1, 2016
Disable send-entity-body-document.htm.

We don't implement the feature it tests anyway. Filed #9490 for that.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9491)
<!-- Reviewable:end -->
@fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 1, 2016

Is it OK if I take a stab at this issue?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Feb 1, 2016

Of course!

@KiChjang KiChjang added the C-assigned label Feb 1, 2016
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Mar 12, 2016

@fhahn Are you still working on this?

@jdm
Copy link
Member

@jdm jdm commented Mar 12, 2016

If you forgot about it, may I ask that it remain forgotten? I filed #9963 yesterday without realizing it was a duplicate and I'm hoping some students will start working on it soon.

@fhahn
Copy link
Contributor

@fhahn fhahn commented Mar 22, 2016

Sorry for taking so long to respond. Unfortunately I did not have time to work on this in the last weeks and I won't for a few more weeks. So feel free to give this issue to somebody else :)

@jdm jdm removed the C-assigned label Mar 22, 2016
@jdm
Copy link
Member

@jdm jdm commented Apr 10, 2016

For implementing the serialization of the document, we should be able to use the existing HTML serialization code. See serialize in components/script/dom/element.rs for an example!

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 12, 2016

Hey guys, I was wondering how to do this.
I saw a function named Open on impl XMLHttpRequestMethods for XMLHttpRequest, this would be to implement a Send version?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Apr 12, 2016

@jaysonsantos I'm pretty sure that there's also a Send fn. Can you confirm that you are currently working on this?

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 12, 2016

@KiChjang Ah, I saw it but I don't know what to do. Is there something missing on it?
Also don't put as me doing because I don't know if I am able to do it because I know nothing about servo code.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Apr 12, 2016

The specification points to step 4 of the XMLHttpRequest.send method, which is this code. Right now, it assumes that the data parameter is of BodyInit type; we'll need to check whether it's a Document as well and handle it accordingly. The WebIDL file generating our Rust bindings will also be need to updated, so that data objects of Document type gets properly sent in as a parameter to XHR.send.

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 12, 2016

@KiChjang Thanks I will try to take a look

@KiChjang KiChjang added the C-assigned label Apr 12, 2016
@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 13, 2016

Do you guys have any tip on how to do this? Send is already inside a trait, how could I accept Option<BodyInit> or Option<Document> without using traits?

@jdm
Copy link
Member

@jdm jdm commented Apr 13, 2016

If you make the change to the WebIDL file and rebuild, the resulting Rust build errors should inform how you need to modify the Rust code to accommodate the change.

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 13, 2016

Hey guys I started doing some stuff on my branch in https://github.com/servo/servo/compare/master...jaysonsantos:xhr-send?expand=1 but somehow the Document which is being sent to Send is always a HTML document https://github.com/jaysonsantos/servo/blob/xhr-send/components/script/dom/xmlhttprequest.rs#L1428 despite the test setting the header properly https://github.com/jaysonsantos/servo/blob/xhr-send/tests/wpt/web-platform-tests/XMLHttpRequest/resources/win-1252-xml.py
My question is: is this an issue that should be investigated or am I just missing something?

@jdm
Copy link
Member

@jdm jdm commented Apr 13, 2016

That's probably our XML parser not being invoked properly when parsing the document, which is being tracked in #10581, so you can leave that as an expected failure for the purposes of this PR.

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 14, 2016

Probably that is happening everywhere because for all tests document.CharacterSet() returns UTF-8.
I put some debug messages here https://github.com/servo/servo/compare/master...jaysonsantos:xhr-send?expand=1#diff-016eef304e69674de212b7fbcd9330fdR1433
And the response is always this:

DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "win-1252-xml.py"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "invalid-utf8-html.py"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "shift-jis-html.py"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "folder.txt"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "img-utf8-html.py"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "empty-div-utf8-html.py"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
DEBUG:script::dom::xmlhttprequest: Document is already utf-8, skipping conversion Url { scheme: "http", scheme_data: Relative(RelativeSchemeData { username: "", password: None, host: Domain("web-platform.test"), port: Some(8000), default_port: Some(80), path: ["XMLHttpRequest", "resources", "image.gif"] }), query: None, fragment: None }
DEBUG:script::dom::xmlhttprequest: request_headers = Headers { Referer: http://web-platform.test:8000/XMLHttpRequest/send-entity-body-document.htm, }
@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 14, 2016

Should I also disable those tests?

@jdm
Copy link
Member

@jdm jdm commented Apr 14, 2016

Well, it's not disabling the tests, it's just marking them as not passing, presumably.

@jaysonsantos
Copy link

@jaysonsantos jaysonsantos commented Apr 14, 2016

Yes, what I mean is, should I submit the PR with non utf8 tests marked as failed? As I saw Document::new is always creating it with UTF-8.

@jdm
Copy link
Member

@jdm jdm commented Apr 14, 2016

Yes, that sounds reasonable to me.

@jdm jdm added the C-has open PR label Apr 29, 2016
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Oct 12, 2016

Most recent attempt at #10604.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 8, 2016

@cynicaldevil is currently working on this.

@KiChjang KiChjang added the C-assigned label Dec 8, 2016
bors-servo added a commit that referenced this issue Dec 20, 2016
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 23, 2016
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 24, 2016
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
@jdm jdm added the C-has open PR label Jan 5, 2017
bors-servo added a commit that referenced this issue Jan 7, 2017
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 8, 2017
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 8, 2017
Implemented XMLHttpRequest.send(Document)

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #9490 .

<!-- Either: -->
r? @KiChjang

`XMLHttpRequest/send-entity-body-document.htm` is not working atm, I get this output:
```
$ ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm
Running 1 tests in web-platform-tests

Ran 1 tests finished in 0.0 seconds.
  • 1 ran as expected. 1 tests skipped.
```
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14648)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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