commit 3417e4afce5eaff498a875c7ea4f7f964bbdec6a
parent 2b26018c95b37e1ac0208e2e7f3f5455592e9a39
Author: Kagami Sascha Rosylight <krosylight@proton.me>
Date: Thu, 20 Nov 2025 09:20:09 +0000
Bug 2000862 - Part 1: Remove the dom.webnotifications.icon_encoding_utf8.enabled pref r=webidl,smaug,tschuster
Differential Revision: https://phabricator.services.mozilla.com/D273196
Diffstat:
7 files changed, 11 insertions(+), 130 deletions(-)
diff --git a/dom/notification/Notification.cpp b/dom/notification/Notification.cpp
@@ -10,7 +10,6 @@
#include "Navigator.h"
#include "NotificationUtils.h"
-#include "mozilla/Encoding.h"
#include "mozilla/HoldDropJSObjects.h"
#include "mozilla/OwningNonNull.h"
#include "mozilla/StaticPrefs_dom.h"
@@ -565,8 +564,8 @@ uint32_t Notification::MaxActions(const GlobalObject& aGlobal) {
}
nsresult Notification::ResolveIconURL(nsIGlobalObject* aGlobal,
- const nsAString& aIconUrl,
- nsString& aDecodedUrl) {
+ const nsACString& aIconUrl,
+ nsString& aResolvedUrl) {
nsresult rv = NS_OK;
if (aIconUrl.IsEmpty()) {
@@ -578,70 +577,16 @@ nsresult Notification::ResolveIconURL(nsIGlobalObject* aGlobal,
return rv;
}
- auto encoding = UTF_8_ENCODING;
-
- // TODO(krosylight): Ultimately we can use UTF8String for icon definition of
- // Web IDL when we remove this.
- if (!StaticPrefs::dom_webnotifications_icon_encoding_utf8_enabled()) {
- if (nsCOMPtr<nsPIDOMWindowInner> window = aGlobal->GetAsInnerWindow()) {
- if (RefPtr<Document> doc = window->GetExtantDoc()) {
- encoding = doc->GetDocumentCharacterSet();
- } else {
- NS_WARNING("No document found for main thread notification!");
- return NS_ERROR_FAILURE;
- }
- }
- }
-
nsCOMPtr<nsIURI> srcUri;
- rv = NS_NewURI(getter_AddRefs(srcUri), aIconUrl, encoding, baseUri);
+ rv = NS_NewURI(getter_AddRefs(srcUri), aIconUrl, nullptr, baseUri);
if (NS_SUCCEEDED(rv)) {
nsAutoCString src;
srcUri->GetSpec(src);
// XXX(krosylight): We should be able to pass UTF8 as-is, or ideally the URI
// object itself.
- CopyUTF8toUTF16(src, aDecodedUrl);
+ CopyUTF8toUTF16(src, aResolvedUrl);
}
- // TODO(krosylight): We should be able to remove the following when removing
- // the non-UTF8 branch above.
- if (encoding == UTF_8_ENCODING) {
- return rv;
- }
-
- // If it was not UTF8, let's try UTF8 and see whether the result differs. If
- // no difference is found then we can just use UTF8 everywhere.
- // See: https://github.com/whatwg/notifications/issues/209
- glean::web_notification::IconUrlEncodingLabel label =
- glean::web_notification::IconUrlEncodingLabel::eNeitherWay;
-
- nsCOMPtr<nsIURI> srcUriUtf8;
- nsresult rvUtf8 =
- NS_NewURI(getter_AddRefs(srcUriUtf8), aIconUrl, UTF_8_ENCODING, baseUri);
-
- if (NS_SUCCEEDED(rv)) {
- if (NS_SUCCEEDED(rvUtf8)) {
- bool equals = false;
- if (NS_SUCCEEDED(srcUri->Equals(srcUriUtf8, &equals))) {
- if (equals) {
- // Okay to be parsed with UTF8
- label = glean::web_notification::IconUrlEncodingLabel::eUtf8;
- } else {
- // Can be parsed either way but with difference, unclear which one is
- // intended without fetching
- label = glean::web_notification::IconUrlEncodingLabel::eEitherWay;
- }
- }
- } else {
- label = glean::web_notification::IconUrlEncodingLabel::eDocumentCharset;
- }
- } else if (NS_SUCCEEDED(rvUtf8)) {
- // Can be only parsed with UTF8
- label = glean::web_notification::IconUrlEncodingLabel::eUtf8;
- }
-
- glean::web_notification::icon_url_encoding.EnumGet(label).Add();
-
return rv;
}
diff --git a/dom/notification/Notification.h b/dom/notification/Notification.h
@@ -13,6 +13,7 @@
#include "mozilla/dom/notification/NotificationChild.h"
#include "nsCycleCollectionParticipant.h"
#include "nsISupports.h"
+#include "nsString.h"
class nsIPrincipal;
class nsIVariant;
@@ -98,8 +99,8 @@ class Notification : public DOMEventTargetHelper, public SupportsWeakPtr {
aRetval = mIPCNotification.options().tag();
}
- void GetIcon(nsAString& aRetval) {
- aRetval = mIPCNotification.options().icon();
+ void GetIcon(nsACString& aRetval) {
+ aRetval = NS_ConvertUTF16toUTF8(mIPCNotification.options().icon());
}
void MaybeNotifyClose();
@@ -193,8 +194,8 @@ class Notification : public DOMEventTargetHelper, public SupportsWeakPtr {
bool SendShow(Promise* aPromise);
static nsresult ResolveIconURL(nsIGlobalObject* aGlobal,
- const nsAString& aIconURL,
- nsString& aResolvedURL);
+ const nsACString& aIconUrl,
+ nsString& aResolvedUrl);
};
} // namespace mozilla::dom
diff --git a/dom/notification/metrics.yaml b/dom/notification/metrics.yaml
@@ -67,19 +67,3 @@ web_notification:
notification_emails:
- krosylight@mozilla.com
expires: never
- icon_url_encoding:
- type: labeled_counter
- description: >
- The encoding of the notification icon URL
- labels:
- - utf8
- - document_charset
- - either_way
- - neither_way
- bugs:
- - https://bugzilla.mozilla.org/show_bug.cgi?id=1914203
- data_reviews:
- - https://bugzilla.mozilla.org/show_bug.cgi?id=1914203
- notification_emails:
- - krosylight@mozilla.com
- expires: 156
diff --git a/dom/notification/test/mochitest/mochitest.toml b/dom/notification/test/mochitest/mochitest.toml
@@ -34,8 +34,6 @@ support-files = ["blank.html"]
["test_notification_crossorigin_iframe_nested_glean.html"]
support-files = ["blank.html"]
-["test_notification_euc_kr.html"]
-
["test_notification_insecure_context.html"]
# This test needs to be run on HTTP (not HTTPS).
skip-if = [
diff --git a/dom/notification/test/mochitest/test_notification_euc_kr.html b/dom/notification/test/mochitest/test_notification_euc_kr.html
@@ -1,41 +0,0 @@
-<!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 SpecialPowers.pushPrefEnv({
- set: [["dom.webnotifications.icon_encoding_utf8.enabled", false]]
- });
-
- 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>
diff --git a/dom/webidl/Notification.webidl b/dom/webidl/Notification.webidl
@@ -50,7 +50,7 @@ interface Notification : EventTarget {
readonly attribute DOMString? tag;
[Pure]
- readonly attribute DOMString? icon;
+ readonly attribute UTF8String? icon;
[Constant, Pref="dom.webnotifications.requireinteraction.enabled"]
readonly attribute boolean requireInteraction;
@@ -79,7 +79,7 @@ dictionary NotificationOptions {
DOMString lang = "";
DOMString body = "";
DOMString tag = "";
- DOMString icon = "";
+ UTF8String icon = "";
boolean requireInteraction = false;
boolean silent = false;
VibratePattern vibrate;
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -5775,12 +5775,6 @@
#endif
mirror: always
-# Always treat icon URLs as UTF8 instead of document charset
-- name: dom.webnotifications.icon_encoding_utf8.enabled
- type: RelaxedAtomicBool
- value: true
- mirror: always
-
# Forcing all notification requests do storage cleanup.
- name: dom.webnotifications.testing.force_storage_cleanup.enabled
type: bool
