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 upTreat application/xml like text/xml in ParserContext::process_response #15850
Labels
Comments
|
Related: #14912 |
|
Hi @jdm , I’ve gone through mod.rs, to see how the different mime contexts are being considered in order to serve a response. What I understand from everything written above is that application/xml files must be treated in the same way that text/xml files are currently being treated. However, in the file mod.rs, at line 587, I see is that nothing is done once we encounter text/xml. I'm not sure of how to proceed from here and I'd really appreciate some pointers. |
|
Right, the decision for text/xml is "we don't need to do anything special; just process the rest of the page content as usual". The next branch in that match handles any content types that we don't recognize and makes us replace the content with a generic error. |
bors-servo
added a commit
that referenced
this issue
Apr 11, 2017
Included block to handle application/xml <!-- Please describe your changes on the following line: --> Hey @jdm, based on my understanding of the issue, I have added a new branch in the match to handle application/xml content type. So now when the block encounters application/xml, it does nothing special and proceeds to process the rest of the page as usual. The "Unknown content type" message is no longer displayed when application/xml is encountered. Is this the update that was expected? It seems like a very simple fix and I was wondering if there was something that I was missing out on doing (like adding a new and more relevant message on encountering application/xml) --- <!-- 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 #15850 (github issue number if applicable). <!-- Either: --> - [ ] 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. --> <!-- 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/16341) <!-- Reviewable:end -->
|
Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the If you intend to work on this issue, then add |
|
The code change is straightforward, and then there are a bunch of test results that need to be updated as discussed in #16341. |
|
I'll take it if nobody is already on it :) |
|
Hey @rigelk! Thanks for your interest in working on this issue. It's now assigned to you! |
rigelk
added a commit
to rigelk/servo
that referenced
this issue
Jun 22, 2017
- components/script/dom/servoparser/mod.rs: merged all xml-related branches and added application/xml - tests updated to expect the new result Fixes servo#15850
rigelk
added a commit
to rigelk/servo
that referenced
this issue
Jun 22, 2017
- components/script/dom/servoparser/mod.rs: merged all xml-related branches and added application/xml - tests updated to expect the new result Fixes servo#15850
rigelk
added a commit
to rigelk/servo
that referenced
this issue
Jun 22, 2017
- components/script/dom/servoparser/mod.rs: merged all xml-related branches and added application/xml - tests updated to expect the new result Fixes servo#15850
bors-servo
added a commit
that referenced
this issue
Jun 23, 2017
Treat application/xml like text/xml in ParserContext::process_response - components/script/dom/servoparser/mod.rs: merged existing xml-related branches into one and added application/xml as a supported type in that branch. - tests updated to expect the new result Fixes #15850 <!-- 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/17474) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Jun 30, 2017
Treat application/xml like text/xml in ParserContext::process_response - components/script/dom/servoparser/mod.rs: merged existing xml-related branches into one and added application/xml as a supported type in that branch. - tests updated to expect the new result Fixes #15850 --- - [X] `./mach build` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes modify expected results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] The tests run consistently <!-- 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/17474) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Jul 18, 2017
Treat application/xml like text/xml in ParserContext::process_response - components/script/dom/servoparser/mod.rs: merged existing xml-related branches into one and added application/xml as a supported type in that branch. - tests updated to expect the new result Fixes #15850 --- - [X] `./mach build` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes modify expected results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] The tests run consistently <!-- 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/17474) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Jul 28, 2017
Treat application/xml like text/xml in ParserContext::process_response - components/script/dom/servoparser/mod.rs: merged existing xml-related branches into one and added application/xml as a supported type in that branch. - tests updated to expect the new result Fixes #15850 --- - [X] `./mach build` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes modify expected results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] The tests run consistently <!-- 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/17474) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Jul 31, 2017
Treat application/xml like text/xml in ParserContext::process_response - components/script/dom/servoparser/mod.rs: merged existing xml-related branches into one and added application/xml as a supported type in that branch. - tests updated to expect the new result Fixes #15850 --- - [X] `./mach build` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes modify expected results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] The tests run consistently <!-- 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/17474) <!-- Reviewable:end -->
|
Last effort was #17474, which just needs some more work to address the unexpected test results. |
|
@jdm could I take a stab at this or would you prefer it be claimed by a RGSoC applicant? |
|
No, please do! |
|
@highfive : assign me |
|
It looks like this has already been assigned to someone. I'll leave the decision to a core contributor. |
|
Ignore @highfive's message; I've already assigned this issue to you. |
|
@KiChjang thanks! |
bors-servo
added a commit
that referenced
this issue
Nov 10, 2017
Treat application/xml like text/xml in ParserContext::process_response <!-- Please describe your changes on the following line: --> * components/script/dom/servoparser/mod.rs updated to handle application/xml as text/xml, along with hoisting application/xhtml+xml from the unknown mime types match arm. * tests updated via mach --- <!-- 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 #15850 (github issue number if applicable). <!-- Either: --> - [X] These changes modify results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] 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. --> <!-- 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/19182) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Nov 11, 2017
Treat application/xml like text/xml in ParserContext::process_response <!-- Please describe your changes on the following line: --> * components/script/dom/servoparser/mod.rs updated to handle application/xml as text/xml, along with hoisting application/xhtml+xml from the unknown mime types match arm. * tests updated via mach --- <!-- 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 #15850 (github issue number if applicable). <!-- Either: --> - [X] These changes modify results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] 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. --> <!-- 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/19182) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Nov 16, 2017
Treat application/xml like text/xml in ParserContext::process_response <!-- Please describe your changes on the following line: --> * components/script/dom/servoparser/mod.rs updated to handle application/xml as text/xml, along with hoisting application/xhtml+xml from the unknown mime types match arm. * tests updated via mach --- <!-- 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 #15850 (github issue number if applicable). <!-- Either: --> - [X] These changes modify results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] 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. --> <!-- 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/19182) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this issue
Nov 18, 2017
Treat application/xml like text/xml in ParserContext::process_response <!-- Please describe your changes on the following line: --> * components/script/dom/servoparser/mod.rs updated to handle application/xml as text/xml, along with hoisting application/xhtml+xml from the unknown mime types match arm. * tests updated via mach --- <!-- 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 #15850 (github issue number if applicable). <!-- Either: --> - [X] These changes modify results in test files via the automated: `./mach test-wpt tests/wpt/web-platform-tests/dom/nodes --log-raw /tmp/servo && ./mach update-wpt /tmp/servo` - [ ] 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. --> <!-- 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/19182) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We should be able to support loading documents with the application/xml MIME type, but currently we just show an error instead. This appears in
./mach test-wpt tests/wpt/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm; we will get more meaningful errors if we make this change.Code:
components/script/dom/servoparser/mod.rsClaiming this for some RGSoC applicants.