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.responseURL #8830

Closed
jdm opened this issue Dec 4, 2015 · 12 comments
Closed

Implement XMLHttpRequest.responseURL #8830

jdm opened this issue Dec 4, 2015 · 12 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Dec 4, 2015

We have a method that ostensibly implements the specification (XMLHttpRequest::ResponseURL), but we don't actually set the response_url member of XMLHttpRequest anywhere except the constructor. We should set this value appropriately inside process_partial_response in the XHRProgress::Done branch, and the member will need to be DOMRefCell<DOMString> (rather than a plain DOMString) to accommodate the new mutation.

Code: components/script/dom/xmlhttprequest.rs
Tests: ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest

@jackinloadup
Copy link

@jackinloadup jackinloadup commented Dec 11, 2015

I'd like to try and take this task on

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 11, 2015

You got it!

@KiChjang KiChjang added the C-assigned label Dec 11, 2015
@jdm
Copy link
Member Author

@jdm jdm commented Dec 11, 2015

Oh, I forgot to include the specification in the original description: https://xhr.spec.whatwg.org/#the-responseurl-attribute

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 29, 2015

@jackinloadup any updates on this?

@jackinloadup
Copy link

@jackinloadup jackinloadup commented Dec 31, 2015

I have some code in place but I have been having some difficulty figuring out how to debug the internals of various values.

Namely how to use gdb or some other tool to inspect aspects of hyper headers or url fields.

jackinloadup@be1d04f

@jdm
Copy link
Member Author

@jdm jdm commented Dec 31, 2015

@jackinloadup There are rust-lldb and rust-gdb wrappers that include pretty-printers in .servo/rust/[hash]/rustc-1.0.0-[platform]/bin/ that can be run against the target/debug/servo binary.

@jdm
Copy link
Member Author

@jdm jdm commented Dec 31, 2015

I use regular lldb on OS X for the same purpose, via ./mach run --debug.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Jan 21, 2016

@jackinloadup still working on this issue?

@jdm jdm removed the C-assigned label Jan 26, 2016
@shinglyu
Copy link
Member

@shinglyu shinglyu commented Jan 29, 2016

May I give it a try?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Jan 29, 2016

Yes, absolutely!

@KiChjang KiChjang added the C-assigned label Jan 29, 2016
@shinglyu
Copy link
Member

@shinglyu shinglyu commented Feb 2, 2016

I tried to write a wpt test to cover this. But I was unable to run the single test file.
STR:

  • Use ./mach create-wpt tests/wpt/web-platform-tests/XMLHttpRequest/responseurl.htm
  • Copy the responsexml-basic.htm test into responseurl.htm
  • Run with ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/responseurl.htm

Expected:
The test runs

Actual:
The test starts, opens a blak Servo window and hangs forever

Notes:
I tried ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest and it works, no Servo window poping up, and the test can catch errors if I add an assert(false) in it. But running a single file (I tried ./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/responsexml-basic.htm) it just doesn't run. What's the difference between running a single file and a folder?

@jdm
Copy link
Member Author

@jdm jdm commented Feb 2, 2016

That's very strange; that's supposed to work. Could you push your changes to a branch on github?

bors-servo added a commit that referenced this issue Feb 3, 2016
Implemented XMLHttpRequest.responseURL

Resolves #8830

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9518)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 22, 2016
Implemented XMLHttpRequest.responseURL

Resolves #8830

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9518)
<!-- 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.

None yet
4 participants
You can’t perform that action at this time.