Update overrideMimeType and final-charset to match xhr spec #29812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@bors-servo try=wpt |
bors-servo
added a commit
that referenced
this pull request
May 29, 2023
Update overrideMimeType and final-encoding to match xhr spec While looking into issues related to `overridemimetype`, I noticed it doesn't match the spec because the method is updated several times in the past few years. But, there are still remaining failures for tests in `xhr/overridemimetype-blob.html` so this PR aims to match the spec first and investigate remaining failures in a separate PR. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable) - [x] There are tests for these changes
CYBAI
changed the title
|
Test results for linux-wpt-layout-2013 from try job (#5112992342): Flaky unexpected result (17)
Stable unexpected results that are known to be intermittent (19)
Stable unexpected results (9)
|
|
💔 Test failed - checks-github |
|
I will look into the unexpected TIMEOUTs and failures 👀 |
|
@bors-servo try=wpt |
|
🔨 Triggering try run (#6198188414) with platform=linux and layout=2013 |
|
Kicking off a new try job in the case that the timeouts are actually panics. |
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
|
Test results for linux-wpt-layout-2013 from try job (#6198188414): Flaky unexpected result (19)
Stable unexpected results that are known to be intermittent (16)
Stable unexpected results (9)
|
|
|
|
Here is the information about the timeouts and errors: It looks like there is a bit of work to do on the implementation of this feature. |
mrobinson
force-pushed
the
xhr-overridemimetype
branch
from
49c1197
to
b64f934
Compare
|
🔨 Triggering try run (#8085114293) for Linux WPT |
|
Test results for linux-wpt-layout-2020 from try job (#8085114293): Flaky unexpected result (21)
Stable unexpected results that are known to be intermittent (11)
Stable unexpected results (4)
|
|
|
mrobinson
force-pushed
the
xhr-overridemimetype
branch
from
b64f934
to
8456b74
Compare
|
I think this is ready to go with a small fix: commit 8456b742333aa396692690a69c9feafc39b11fd5
Author: Martin Robinson <mrobinson@igalia.com>
Date: Wed Feb 28 19:10:16 2024 +0100
Fix an issue and add more comments
diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs
index 1f5977299e..e748d23d6b 100644
--- a/components/script/dom/xmlhttprequest.rs
+++ b/components/script/dom/xmlhttprequest.rs
@@ -889,9 +889,10 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
ByteString::new(v)
}
- // https://xhr.spec.whatwg.org/#the-overridemimetype()-method
+ /// <https://xhr.spec.whatwg.org/#the-overridemimetype()-method>
fn OverrideMimeType(&self, mime: DOMString) -> ErrorResult {
- // Step 1
+ // 1. If this’s state is loading or done, then throw an "InvalidStateError"
+ // DOMException.
match self.ready_state.get() {
XMLHttpRequestState::Loading | XMLHttpRequestState::Done => {
return Err(Error::InvalidState);
@@ -899,7 +900,9 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
_ => {},
}
- // Step 2-3.
+ // 2. Set this’s override MIME type to the result of parsing mime.
+ // 3. If this’s override MIME type is failure, then set this’s override MIME type
+ // to application/octet-stream.
let override_mime = match mime.parse::<Mime>() {
Ok(mime) => mime,
Err(_) => "application/octet-stream"
@@ -908,7 +911,6 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
};
*self.override_mime_type.borrow_mut() = Some(override_mime);
-
Ok(())
}
@@ -1598,20 +1600,26 @@ impl XMLHttpRequest {
/// <https://xhr.spec.whatwg.org/#final-charset>
fn final_charset(&self) -> Option<&'static Encoding> {
- // Step 2-3.
+ // 1. Let label be null.
+ // 2. Let responseMIME be the result of get a response MIME type for xhr.
+ // 3. If responseMIME’s parameters["charset"] exists, then set label to it.
let response_charset = self
.response_mime_type()
.and_then(|mime| mime.get_param(mime::CHARSET).map(|c| c.to_string()));
- // Step 4.
+ // 4. If xhr’s override MIME type’s parameters["charset"] exists, then set label to it.
let override_charset = self
.override_mime_type
.borrow()
.as_ref()
.and_then(|mime| mime.get_param(mime::CHARSET).map(|c| c.to_string()));
- response_charset
- .or(override_charset)
+ // 5. If label is null, then return null.
+ // 6. Let encoding be the result of getting an encoding from label.
+ // 7. If encoding is failure, then return null.
+ // 8. Return encoding.
+ override_charset
+ .or(response_charset)
.and_then(|charset| Encoding::for_label(&charset.as_bytes()))
}
|
mrobinson
approved these changes
Feb 28, 2024
mrobinson
force-pushed
the
xhr-overridemimetype
branch
from
8456b74
to
d48d1fd
Compare
|
Martin, thanks for fixing this 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While looking into issues related to
overridemimetype, I noticed it doesn't match the spec because the method is updated several times in the past few years.But, there are still remaining failures for tests in
xhr/overridemimetype-blob.htmlso this PR aims to match the spec first and investigate remaining failures in a separate PR../mach build -ddoes not report any errors./mach test-tidydoes not report any errors