-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(core) Fix HTML encoding in webview rendered via data url #8779
base: 1.x
Are you sure you want to change the base?
Conversation
5b58798
to
22c0674
Compare
22c0674
to
2f82563
Compare
683c540
to
f91ab78
Compare
6166e42
to
7cdf241
Compare
webview_builder = if let Some(html_string) = tauri_utils::html::extract_html_content(&url) { | ||
webview_builder | ||
.with_html(html_string) | ||
.map_err(|e| Error::CreateWebview(Box::new(e)))? | ||
} else { | ||
webview_builder | ||
.with_url(&url) | ||
.map_err(|e| Error::CreateWebview(Box::new(e)))? | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tauri-runtime-wry
shouldn't do these checks at all, instead, PendingWindow
should have a field html: Option<String>
and if it is Some
, we use with_html
@@ -21,6 +21,7 @@ rand = "0.8" | |||
raw-window-handle = "0.5" | |||
tracing = { version = "0.1", optional = true } | |||
arboard = { version = "3", optional = true } | |||
percent-encoding = "2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only needed on Linux, let's revert this back
@@ -50,3 +50,4 @@ system-tray = [ ] | |||
macos-private-api = [ ] | |||
global-shortcut = [ ] | |||
clipboard = [ ] | |||
window-data-url = [ "tauri-utils/window-data-url" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and revert this as well
@@ -38,6 +38,7 @@ semver = "1" | |||
infer = "0.13" | |||
dunce = "1" | |||
log = "0.4.20" | |||
data-url = { version = "0.3.1", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -54,3 +55,4 @@ process-relaunch-dangerous-allow-symlink-macos = [ ] | |||
config-json5 = [ "json5" ] | |||
config-toml = [ "toml" ] | |||
resources = [ "glob", "walkdir" ] | |||
window-data-url = [ "data-url" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
/// Temporary naive method to check if a string is a html | ||
pub fn is_html(data_string: &str) -> bool { | ||
data_string.contains('<') && data_string.contains('>') | ||
} | ||
|
||
/// Temporary naive method to extract data from html data string | ||
pub fn extract_html_content(input: &str) -> Option<&str> { | ||
if input.starts_with("data:text/html,") { | ||
Some(&input[15..]) | ||
} else { | ||
None | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move these back to tauri
crate as they are only used there for now
if html.contains('<') && html.contains('>') { | ||
match ( | ||
url.scheme(), | ||
tauri_utils::html::extract_html_content(url.as_str()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't call extract_html_content
unless we are sure it is a data url
#[cfg(feature = "window-data-url")]
if ur.scheme() == "data" {
let html = extract_html_content();
// ...
}
// There is an issue with the external DataUrl where HTML containing special characters | ||
// are not correctly processed. A workaround is to first percent encode the html string, | ||
// before it processed by DataUrl. | ||
let encoded_string = percent_encoding::utf8_percent_encode(html_string, percent_encoding::NON_ALPHANUMERIC).to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't encode the URL, we should expect the user has already encoded it as per https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs
which makes this PR kinda uneeded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that data URLs are not encoded by default on Linux, but they are encoded by default on macOS. I haven't tried on Windows yet. I expect consistency across platforms, with no encoding.
The following content will be encoded on macOS, which is not what I expected.
const webview = new WebviewWindow('print', {
url: `data:text/html,<html><body>你好,世界</body></html>`,
center: true,
visible: true,
width: 300,
height: 300,
});
closes #8760 in 1.x