corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1961872 - Fix the logic for icon encoding telemetry r=asuth

Browse Source 
Well, how did I even manage to mess it up this much. I should really write tests for all telemetries.

Basically it was just comparing wrong URLs.

Differential Revision: https://phabricator.services.mozilla.com/D246298
This commit is contained in:
Kagami Sascha Rosylight
2025-04-23 20:51:33 +00:00
parent e74204e9c7
commit 3683f1b7da
4 changed files with 48 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
dom/notification/Notification.cpp
View File

@@ -404,8 +404,8 @@ already_AddRefed<Notification> Notification::ValidateAndCreate(
// Step 12: If options["icon"] exists, then parse it using baseURL, and if
// that does not return failure, set notification’s icon URL to the return
// value. (Otherwise icon URL is not set.)
nsString iconUrl = aOptions.mIcon;
ResolveIconURL(aGlobal, iconUrl);
nsAutoString iconUrl;
ResolveIconURL(aGlobal, aOptions.mIcon, iconUrl);
// Step 19: Set notification’s actions to « ».
nsTArray<IPCNotificationAction> actions;
@@ -573,7 +573,8 @@ uint32_t Notification::MaxActions(const GlobalObject& aGlobal) {
}
nsresult Notification::ResolveIconURL(nsIGlobalObject* aGlobal,
nsString& aIconUrl) {
const nsAString& aIconUrl,
nsString& aDecodedUrl) {
nsresult rv = NS_OK;
if (aIconUrl.IsEmpty()) {
@@ -613,7 +614,8 @@ nsresult Notification::ResolveIconURL(nsIGlobalObject* aGlobal,
if (NS_SUCCEEDED(rv)) {
nsAutoCString src;
srcUri->GetSpec(src);
CopyUTF8toUTF16(src, aIconUrl);
// XXX(krosylight): We should be able to pass UTF8 as-is, or ideally the URI object itself.
CopyUTF8toUTF16(src, aDecodedUrl);
}
if (encoding == UTF_8_ENCODING) {
@@ -628,12 +630,12 @@ nsresult Notification::ResolveIconURL(nsIGlobalObject* aGlobal,
nsCOMPtr<nsIURI> srcUriUtf8;
nsresult rvUtf8 =
NS_NewURI(getter_AddRefs(srcUri), aIconUrl, UTF_8_ENCODING, baseUri);
NS_NewURI(getter_AddRefs(srcUriUtf8), aIconUrl, UTF_8_ENCODING, baseUri);
if (NS_SUCCEEDED(rv)) {
if (NS_SUCCEEDED(rvUtf8)) {
bool equals = false;
if (NS_SUCCEEDED(baseUri->Equals(srcUri, &equals))) {
if (NS_SUCCEEDED(srcUri->Equals(srcUriUtf8, &equals))) {
if (equals) {
// Okay to be parsed with UTF8
label = glean::web_notification::IconUrlEncodingLabel::eUtf8;

2
dom/notification/Notification.h
View File

@@ -194,7 +194,7 @@ class Notification : public DOMEventTargetHelper, public SupportsWeakPtr {
bool CreateActor();
bool SendShow(Promise* aPromise);
static nsresult ResolveIconURL(nsIGlobalObject* aGlobal, nsString& aIconURL);
static nsresult ResolveIconURL(nsIGlobalObject* aGlobal, const nsAString& aIconURL, nsString& aResolvedURL);
};
} // namespace mozilla::dom

3
dom/notification/test/mochitest/mochitest.toml
View File

@@ -20,9 +20,10 @@ support-files = ["blank.html"]
["test_notification_crossorigin_iframe_nested_glean.html"]
support-files = ["blank.html"]
# This test needs to be run on HTTP (not HTTPS).
["test_notification_euc_kr.html"]
["test_notification_insecure_context.html"]
# This test needs to be run on HTTP (not HTTPS).
skip-if = [
"http3",
"http2",

37
dom/notification/test/mochitest/test_notification_euc_kr.html Normal file
View File

@@ -0,0 +1,37 @@
<!DOCTYPE html>
<meta charset="euc-kr">
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/GleanTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
<script>
// This file is actually ascii, because otherwise text editors would have hard time.
// "icon" in Korean, encoded with euc-kr
const iconEucKr = new Uint8Array([0xbe, 0xc6, 0xc0, 0xcc, 0xc4, 0xdc]);
add_task(async function test() {
await GleanTest.testResetFOG();
new Notification("title", { icon: "?icon=icon" });
// "utf8" because the string can be encoded in either euc-kr or utf-8 and
// the result is same.
is(await GleanTest.webNotification.iconUrlEncoding.utf8.testGetValue(), 1, "utf8");
await GleanTest.testResetFOG();
const iconKorean = new TextDecoder("euc-kr").decode(iconEucKr);
const icon = new Notification("title", { icon: `?icon=${iconKorean}` }).icon;
ok(icon.endsWith("?icon=%BE%C6%C0%CC%C4%DC"), "icon encoded with euc-kr");
// "either way" because the string can be encoded in either euc-kr or utf-8
// while the result is different.
is(await GleanTest.webNotification.iconUrlEncoding.either_way.testGetValue(), 1, "either way");
// I cannot find an example to trigger "document charset" or "neither way".
// Probably they can't happen because
// 1. unicode really covers everything that legacy encodings cover
// 2. the URL encoding fall back to HTML entity encoding when document
// charset doesn't support given characters
})
</script>