Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRetrieving a File from FormData isn't getting back the same object #24939
Comments
|
Something is off here: https://xhr.spec.whatwg.org/#interface-formdata can only take a string or blob, so passing it a File must be serializing to a string, and File isn't specified as having any sort of deserialization cache. The test is using assert_array_equals, which ends up in same_value, which ends up at x === y, so it is expecting the actual same File object. |
|
What I was overlooking in the above analysis is that File inherits from Blob, so this should be going into the blob path. First step in investigation: see which path it actually is going into! |
|
https://xhr.spec.whatwg.org/#create-an-entry "Set value to a new File object, representing the same bytes". Even down this path the result should be a different File object! |
|
Based on other browser behavior, I think I see what the mistake is. In the append steps, "filename if given" doesn't mean "if this is the three-argument version of the IDL method", it means "if this is the three-argument version of the IDL method and actually received three arguments". We need a new codepath for only receiving two actual arguments. |
bors-servo
added a commit
that referenced
this issue
Jan 23, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 23, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 23, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 23, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 24, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 24, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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
added a commit
that referenced
this issue
Jan 24, 2020
Make File objects roundtrip through FormData FormData.append now only creates a new File object if it needs to file-wrap a blob or set a filename, and not when appending an existing File with no filename override. The hardest part here was reading the specification in a way that matched browser behavior. --- <!-- 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 #24939 - [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. -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
WPT xhr/formdata-foreach.html puts a File object into a FormData, iterates the FormData, and gets a File object back at the right point in the iteration, but the File object it gets back is not Javascript-equal-to the one it put in, failing the test.