Repository navigation
Improve WebResponse charset handling - #748
Merged
Merged
Conversation
This commit fixes an issue where WebResponse.getCharset() fails to correctly sniff certain charsets in content-type meta tag due to missing support for charset aliases in EncodingSniffer.toCharset().
This commit fixes the priority of the BOM charset and moves it to the highest level to be in line with html/js/css etc. specs.
This commit deprecates WebResponse.defaultCharsetUtf8() and adds a more flexible WebRequest.setDefaultResponseContentCharset() that will be used in the proceeding refactorization commits.
This commit changes the default charset of JavaScript to utf-8 to be in line with official specs.
This commit changes the default charset of CSS to utf-8 to be in line with official specs.
This commit changes the prescan length of HTML from 4096 bytes to 1024 to behave similarly to modern browsers.
This commit changes iframes to be more in line with specs by changing it to use the container document's charset as the fallback charset when a charset is not specified by the iframe document.
This commit moves the BOM and content-type header charset reading code to inside WebResponse.getContentCharsetOrNull() rather than EncodingSniffer since it is common code, and for better flexibility in the proceeding feature commits.
This commit remove the hard to understand getContentCharsetOrNull() and instead adds wasContentCharsetTentative() used to check if the charset returned by getContentCharset() was tenatative. This commit also adds getHeaderContentCharset() as a utility method.
Contributor
Author
|
@rbri I created the PR again |
Member
|
Thanks... |
Member
|
Finally many thanks for that. @duonglaiquang please have a look at the latest code, maybe there are still some problems for you that we have to fix. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR does the following
EncodingSniffer.toCharset().WebRequest.defaultResponseContentCharset_to be used to set the charset for theWebResponsewhen no charset is found. Also deprecatesWebResponse.defaultUtf8().UTF-8.<iframe>so that the parent charset is used when no charset could otherwise be found.WebResponsefromISO-8859-1toUTF-8to be in line with the latest standards.4096to1024bytes to be in line with specs.x-user-definedcharset declaration in meta tag aswindows-1252as documented in specs.@charsetdeclaration.WebResponse.getContentCharset()so that it "just works" for most cases.EncodingSniffertoWebResponseso thatWebResponsehas first-hand knowledge of these concepts.getContentCharsetOrNull()and instead addwasContentCharsetTentative()used to check if the charset returned bygetContentCharset()was "tenatative".WebResponse.getHeaderContentCharset()to cover the limited use-case thatgetContentCharsetOrNull()sort of handled.