From 00e0ca2c08bc1fbbda4149272883fbeed533083a Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Fri, 15 Sep 2023 17:42:46 +0300 Subject: [PATCH] Backed out changeset 106a8fb08a87 (bug 1641389) for causing bc failures on browser_persist_image_accept.js. --- .../DownloadsViewableInternally.sys.mjs | 12 ++++- .../unit/test_DownloadsViewableInternally.js | 51 ++++++++++++++++--- .../browser/browser_persist_image_accept.js | 1 + image/DecoderFactory.cpp | 3 +- image/build/nsImageModule.cpp | 4 ++ image/test/gtest/Common.cpp | 6 ++- .../mochitest/test_animation_operators.html | 3 +- .../mochitest/test_discardAnimatedImage.html | 1 + layout/build/components.conf | 1 - modules/libpref/init/StaticPrefList.yaml | 6 +++ netwerk/protocol/http/nsHttpHandler.cpp | 13 +++-- ...ser_download_open_with_internal_handler.js | 4 ++ .../browser_download_skips_dialog.js | 1 + .../browser_launched_app_save_directory.js | 5 +- 14 files changed, 94 insertions(+), 17 deletions(-) diff --git a/browser/components/downloads/DownloadsViewableInternally.sys.mjs b/browser/components/downloads/DownloadsViewableInternally.sys.mjs index f9c77168b1ad..9684e28537f2 100644 --- a/browser/components/downloads/DownloadsViewableInternally.sys.mjs +++ b/browser/components/downloads/DownloadsViewableInternally.sys.mjs @@ -126,8 +126,16 @@ export let DownloadsViewableInternally = { { extension: "webp", mimeTypes: ["image/webp"], - available: true, - managedElsewhere: false, + initAvailable() { + XPCOMUtils.defineLazyPreferenceGetter( + this, + "available", + "image.webp.enabled", + false, + () => DownloadsViewableInternally._updateHandler(this) + ); + }, + // available getter is set by initAvailable() }, { extension: "avif", diff --git a/browser/components/downloads/test/unit/test_DownloadsViewableInternally.js b/browser/components/downloads/test/unit/test_DownloadsViewableInternally.js index 29d88de55af8..07925bc7d505 100644 --- a/browser/components/downloads/test/unit/test_DownloadsViewableInternally.js +++ b/browser/components/downloads/test/unit/test_DownloadsViewableInternally.js @@ -2,12 +2,14 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ const PREF_SVG_DISABLED = "svg.disabled"; +const PREF_WEBP_ENABLED = "image.webp.enabled"; const PREF_AVIF_ENABLED = "image.avif.enabled"; const PDF_MIME = "application/pdf"; const OCTET_MIME = "application/octet-stream"; const XML_MIME = "text/xml"; const SVG_MIME = "image/svg+xml"; const AVIF_MIME = "image/avif"; +const WEBP_MIME = "image/webp"; const { Integration } = ChromeUtils.importESModule( "resource://gre/modules/Integration.sys.mjs" @@ -80,25 +82,32 @@ function checkAll(mime, ext, expected) { } add_task(async function test_viewable_internally() { - Services.prefs.setCharPref(PREF_ENABLED_TYPES, "xml , svg,avif"); + Services.prefs.setCharPref(PREF_ENABLED_TYPES, "xml , svg,avif,webp"); Services.prefs.setBoolPref(PREF_SVG_DISABLED, false); + Services.prefs.setBoolPref(PREF_WEBP_ENABLED, true); Services.prefs.setBoolPref(PREF_AVIF_ENABLED, true); checkAll(XML_MIME, "xml", false); checkAll(SVG_MIME, "svg", false); + checkAll(WEBP_MIME, "webp", false); checkAll(AVIF_MIME, "avif", false); DownloadsViewableInternally.register(); checkAll(XML_MIME, "xml", true); checkAll(SVG_MIME, "svg", true); + checkAll(WEBP_MIME, "webp", true); checkAll(AVIF_MIME, "avif", true); - // Disable xml, and avif, check that avif becomes disabled + // Remove webp so it won't be cleared + Services.prefs.clearUserPref(PREF_BRANCH_WAS_REGISTERED + "webp"); + + // Disable xml, avif and webp, check that avif becomes disabled Services.prefs.setCharPref(PREF_ENABLED_TYPES, "svg"); - // (XML is externally managed) + // (XML is externally managed, and we just cleared the webp pref) checkAll(XML_MIME, "xml", true); + checkPreferInternal(WEBP_MIME, "webp", true); // Avif should be disabled checkAll(AVIF_MIME, "avif", false); @@ -120,13 +129,15 @@ add_task(async function test_viewable_internally() { ); Assert.ok(!shouldView(OCTET_MIME, "exe"), ".exe shouldn't be accepted"); + Assert.ok(!shouldView(WEBP_MIME), "imave/webp should be disabled by pref"); Assert.ok(!shouldView(AVIF_MIME), "image/avif should be disabled by pref"); // Enable, check that everything is enabled again - Services.prefs.setCharPref(PREF_ENABLED_TYPES, "xml,svg,avif"); + Services.prefs.setCharPref(PREF_ENABLED_TYPES, "xml,svg,webp,avif"); checkAll(XML_MIME, "xml", true); checkAll(SVG_MIME, "svg", true); + checkPreferInternal(WEBP_MIME, "webp", true); checkPreferInternal(AVIF_MIME, "avif", true); Assert.ok( @@ -149,6 +160,7 @@ add_task(async function test_viewable_internally() { for (const [mime, ext, action, ask] of [ [XML_MIME, "xml", Ci.nsIHandlerInfo.useSystemDefault, true], [SVG_MIME, "svg", Ci.nsIHandlerInfo.saveToDisk, true], + [WEBP_MIME, "webp", Ci.nsIHandlerInfo.saveToDisk, false], ]) { let handler = MIMEService.getFromTypeAndExtension(mime, ext); handler.preferredAction = action; @@ -163,8 +175,8 @@ add_task(async function test_viewable_internally() { Assert.equal(handler.alwaysAskBeforeHandling, ask); } - // Enable viewable internally, SVG and XML should not be replaced. - Services.prefs.setCharPref(PREF_ENABLED_TYPES, "svg,xml"); + // Enable viewable internally, SVG and XML should not be replaced, WebP should be saved. + Services.prefs.setCharPref(PREF_ENABLED_TYPES, "svg,webp,xml"); Assert.equal( Services.prefs.prefHasUserValue(PREF_BRANCH_PREVIOUS_ACTION + "svg"), @@ -176,6 +188,16 @@ add_task(async function test_viewable_internally() { false, "svg ask should not be stored" ); + Assert.equal( + Services.prefs.getIntPref(PREF_BRANCH_PREVIOUS_ACTION + "webp"), + Ci.nsIHandlerInfo.saveToDisk, + "webp action should be saved" + ); + Assert.equal( + Services.prefs.getBoolPref(PREF_BRANCH_PREVIOUS_ASK + "webp"), + false, + "webp ask should be saved" + ); { let handler = MIMEService.getFromTypeAndExtension(SVG_MIME, "svg"); @@ -209,6 +231,7 @@ add_task(async function test_viewable_internally() { checkShouldView(XML_MIME, "xml", true); checkAll(SVG_MIME, "svg", true); + checkAll(WEBP_MIME, "webp", true); // Disable SVG to test SVG enabled check (depends on the pref) Services.prefs.setBoolPref(PREF_SVG_DISABLED, true); @@ -223,6 +246,21 @@ add_task(async function test_viewable_internally() { checkAll(SVG_MIME, "svg", true); + // Test WebP enabled check (depends on the pref) + Services.prefs.setBoolPref(PREF_WEBP_ENABLED, false); + // Should have restored the settings from above + { + let handler = MIMEService.getFromTypeAndExtension(WEBP_MIME, "webp"); + Assert.equal(handler.preferredAction, Ci.nsIHandlerInfo.saveToDisk); + Assert.equal(!!handler.alwaysAskBeforeHandling, false); + // Clean up + HandlerService.remove(handler); + } + checkAll(WEBP_MIME, "webp", false); + + Services.prefs.setBoolPref(PREF_WEBP_ENABLED, true); + checkAll(WEBP_MIME, "webp", true); + Assert.ok(!shouldView(null, "pdf"), "missing MIME shouldn't be accepted"); Assert.ok(!shouldView(null, "xml"), "missing MIME shouldn't be accepted"); Assert.ok(!shouldView(OCTET_MIME), "unsupported MIME shouldn't be accepted"); @@ -235,4 +273,5 @@ registerCleanupFunction(() => { // Reset to the defaults Services.prefs.clearUserPref(PREF_ENABLED_TYPES); Services.prefs.clearUserPref(PREF_SVG_DISABLED); + Services.prefs.clearUserPref(PREF_WEBP_ENABLED); }); diff --git a/dom/tests/browser/browser_persist_image_accept.js b/dom/tests/browser/browser_persist_image_accept.js index 126f76665a20..ecf3b370a685 100644 --- a/dom/tests/browser/browser_persist_image_accept.js +++ b/dom/tests/browser/browser_persist_image_accept.js @@ -51,6 +51,7 @@ function expectedImageAcceptHeader() { return ( (Services.prefs.getBoolPref("image.avif.enabled") ? "image/avif," : "") + (Services.prefs.getBoolPref("image.jxl.enabled") ? "image/jxl," : "") + + (Services.prefs.getBoolPref("image.webp.enabled") ? "image/webp," : "") + "*/*" ); } diff --git a/image/DecoderFactory.cpp b/image/DecoderFactory.cpp index f291a0934ab2..4ee7fdfd2205 100644 --- a/image/DecoderFactory.cpp +++ b/image/DecoderFactory.cpp @@ -79,7 +79,8 @@ DecoderType DecoderFactory::GetDecoderType(const char* aMimeType) { type = DecoderType::ICON; // WebP - } else if (!strcmp(aMimeType, IMAGE_WEBP)) { + } else if (!strcmp(aMimeType, IMAGE_WEBP) && + StaticPrefs::image_webp_enabled()) { type = DecoderType::WEBP; // AVIF diff --git a/image/build/nsImageModule.cpp b/image/build/nsImageModule.cpp index b945423be2d7..28fb8bb33086 100644 --- a/image/build/nsImageModule.cpp +++ b/image/build/nsImageModule.cpp @@ -60,10 +60,14 @@ nsresult mozilla::image::EnsureModuleInitialized() { mozilla::StaticPrefs::image_avif_enabled, "image/avif"_ns}; static ImageEnablementCookie kJXLCookie = { mozilla::StaticPrefs::image_jxl_enabled, "image/jxl"_ns}; + static ImageEnablementCookie kWebPCookie = { + mozilla::StaticPrefs::image_webp_enabled, "image/webp"_ns}; Preferences::RegisterCallbackAndCall(UpdateContentViewerRegistration, "image.avif.enabled", &kAVIFCookie); Preferences::RegisterCallbackAndCall(UpdateContentViewerRegistration, "image.jxl.enabled", &kJXLCookie); + Preferences::RegisterCallbackAndCall(UpdateContentViewerRegistration, + "image.webp.enabled", &kWebPCookie); mozilla::image::ShutdownTracker::Initialize(); mozilla::image::ImageFactory::Initialize(); diff --git a/image/test/gtest/Common.cpp b/image/test/gtest/Common.cpp index c3eaaebefce5..d0ddbecedfd4 100644 --- a/image/test/gtest/Common.cpp +++ b/image/test/gtest/Common.cpp @@ -39,8 +39,12 @@ AutoInitializeImageLib::AutoInitializeImageLib() { EXPECT_TRUE(NS_IsMainThread()); sImageLibInitialized = true; + // Ensure WebP is enabled to run decoder tests. + nsresult rv = Preferences::SetBool("image.webp.enabled", true); + EXPECT_TRUE(rv == NS_OK); + // Ensure AVIF is enabled to run decoder tests. - nsresult rv = Preferences::SetBool("image.avif.enabled", true); + rv = Preferences::SetBool("image.avif.enabled", true); EXPECT_TRUE(rv == NS_OK); rv = Preferences::SetBool("image.avif.sequence.enabled", true); EXPECT_TRUE(rv == NS_OK); diff --git a/image/test/mochitest/test_animation_operators.html b/image/test/mochitest/test_animation_operators.html index 2d3a6f6d6764..faece370532e 100644 --- a/image/test/mochitest/test_animation_operators.html +++ b/image/test/mochitest/test_animation_operators.html @@ -159,7 +159,8 @@ function runTests() SimpleTest.waitForExplicitFinish(); SimpleTest.requestFlakyTimeout("untriaged"); -runTests(); + +SpecialPowers.pushPrefEnv({"set": [["image.webp.enabled", true]]}, runTests); diff --git a/image/test/mochitest/test_discardAnimatedImage.html b/image/test/mochitest/test_discardAnimatedImage.html index 09bd9372c6a8..d2aad380e084 100644 --- a/image/test/mochitest/test_discardAnimatedImage.html +++ b/image/test/mochitest/test_discardAnimatedImage.html @@ -40,6 +40,7 @@ window.onload = function() { // Enable discarding for the test. SpecialPowers.pushPrefEnv({ 'set':[['image.mem.discardable',true], + ['image.webp.enabled',true], ['image.avif.sequence.enabled',true]] }, runTest); } diff --git a/layout/build/components.conf b/layout/build/components.conf index 38ddcb2e4df6..6068af7666ed 100644 --- a/layout/build/components.conf +++ b/layout/build/components.conf @@ -45,7 +45,6 @@ content_types = [ 'image/pjpeg', 'image/png', 'image/vnd.microsoft.icon', - 'image/webp', 'image/x-icon', 'image/x-ms-bmp', 'image/x-png', diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index c23822b7d3be..a64fc51816bb 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -7011,6 +7011,12 @@ value: false mirror: always +# Whether we attempt to decode WebP images or not. +- name: image.webp.enabled + type: RelaxedAtomicBool + value: true + mirror: always + # Whether we attempt to decode AVIF images or not. - name: image.avif.enabled type: RelaxedAtomicBool diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 9bd9a46bb58b..5df8dac3b05a 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -216,7 +216,11 @@ static nsCString ImageAcceptHeader() { mimeTypes.Append("image/jxl,"); } - mimeTypes.Append("image/webp,*/*"); + if (mozilla::StaticPrefs::image_webp_enabled()) { + mimeTypes.Append("image/webp,"); + } + + mimeTypes.Append("*/*"); return mimeTypes; } @@ -292,6 +296,7 @@ static const char* gCallbackPrefs[] = { "image.http.accept", "image.avif.enabled", "image.jxl.enabled", + "image.webp.enabled", nullptr, }; @@ -1683,9 +1688,9 @@ void nsHttpHandler::PrefsChanged(const char* pref) { } } - const bool imageAcceptPrefChanged = PREF_CHANGED("image.http.accept") || - PREF_CHANGED("image.avif.enabled") || - PREF_CHANGED("image.jxl.enabled"); + const bool imageAcceptPrefChanged = + PREF_CHANGED("image.http.accept") || PREF_CHANGED("image.avif.enabled") || + PREF_CHANGED("image.jxl.enabled") || PREF_CHANGED("image.webp.enabled"); if (imageAcceptPrefChanged) { nsAutoCString userSetImageAcceptHeader; diff --git a/uriloader/exthandler/tests/mochitest/browser_download_open_with_internal_handler.js b/uriloader/exthandler/tests/mochitest/browser_download_open_with_internal_handler.js index c73538fe19d3..6230e2c39144 100644 --- a/uriloader/exthandler/tests/mochitest/browser_download_open_with_internal_handler.js +++ b/uriloader/exthandler/tests/mochitest/browser_download_open_with_internal_handler.js @@ -487,6 +487,10 @@ add_task(async function test_check_open_with_external_then_internal() { */ add_task( async function test_internal_handler_hidden_with_viewable_internally_type() { + await SpecialPowers.pushPrefEnv({ + set: [["image.webp.enabled", true]], + }); + const mimeInfosToRestore = alwaysAskForHandlingTypes({ "binary/octet-stream": "xml", "image/webp": "webp", diff --git a/uriloader/exthandler/tests/mochitest/browser_download_skips_dialog.js b/uriloader/exthandler/tests/mochitest/browser_download_skips_dialog.js index 0b8c816d2efa..8cc9d68a0763 100644 --- a/uriloader/exthandler/tests/mochitest/browser_download_skips_dialog.js +++ b/uriloader/exthandler/tests/mochitest/browser_download_skips_dialog.js @@ -13,6 +13,7 @@ add_task(async function skipDialogAndDownloadFile() { set: [ ["browser.download.always_ask_before_handling_new_types", false], ["browser.download.useDownloadDir", true], + ["image.webp.enabled", true], ], }); diff --git a/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js b/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js index d97841026327..993a4f162b27 100644 --- a/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js +++ b/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js @@ -13,7 +13,10 @@ const TEST_PATH = getRootDirectory(gTestPath).replace( add_setup(async function () { await SpecialPowers.pushPrefEnv({ - set: [["browser.download.always_ask_before_handling_new_types", false]], + set: [ + ["browser.download.always_ask_before_handling_new_types", false], + ["image.webp.enabled", true], + ], }); const allowDirectoriesVal = DownloadIntegration.allowDirectories; DownloadIntegration.allowDirectories = true;