From 7e3ff69dfb7e36c747a3f50d5de75c9d1ee2a0c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 14 Oct 2021 04:26:28 +0000 Subject: [PATCH] Bug 1731940 - Web Manifest: implement id processing r=saschanaz spec change https://github.com/w3c/manifest/pull/988 Differential Revision: https://phabricator.services.mozilla.com/D126331 --- dom/locales/en-US/chrome/dom/dom.properties | 2 + dom/manifest/ManifestProcessor.jsm | 48 ++++++- dom/manifest/test/mochitest.ini | 1 + .../test/test_ManifestProcessor_id.html | 123 ++++++++++++++++++ .../test_ManifestProcessor_start_url.html | 31 ++++- .../test/test_ManifestProcessor_warnings.html | 19 +++ 6 files changed, 213 insertions(+), 11 deletions(-) create mode 100644 dom/manifest/test/test_ManifestProcessor_id.html diff --git a/dom/locales/en-US/chrome/dom/dom.properties b/dom/locales/en-US/chrome/dom/dom.properties index 2117887e4c18e..e7df4a40699e2 100644 --- a/dom/locales/en-US/chrome/dom/dom.properties +++ b/dom/locales/en-US/chrome/dom/dom.properties @@ -212,6 +212,8 @@ ServiceWorkerGraceTimeoutTermination=Terminating ServiceWorker for scope ‘%1$S # LOCALIZATION NOTE (ServiceWorkerNoFetchHandler): Do not translate "Fetch". ServiceWorkerNoFetchHandler=Fetch event handlers must be added during the worker script’s initial evaluation. ExecCommandCutCopyDeniedNotInputDriven=document.execCommand(‘cut’/‘copy’) was denied because it was not called from inside a short running user-generated event handler. +ManifestIdIsInvalid=The id member did not resolve to a valid URL. +ManifestIdNotSameOrigin=The id member must have the same origin as the start_url member. ManifestShouldBeObject=Manifest should be an object. ManifestScopeURLInvalid=The scope URL is invalid. ManifestScopeNotSameOrigin=The scope URL must be same origin as document. diff --git a/dom/manifest/ManifestProcessor.jsm b/dom/manifest/ManifestProcessor.jsm index b551f5c25a700..6830b80b66ac6 100644 --- a/dom/manifest/ManifestProcessor.jsm +++ b/dom/manifest/ManifestProcessor.jsm @@ -124,6 +124,7 @@ var ManifestProcessor = { background_color: processBackgroundColorMember(), }; processedManifest.scope = processScopeMember(); + processedManifest.id = processIdMember(); if (checkConformance) { processedManifest.moz_validation = errors; processedManifest.moz_manifest_url = manifestURL.href; @@ -258,10 +259,10 @@ var ManifestProcessor = { expectedType: "string", trim: false, }; - let result = new URL(docURL).href; + const defaultStartURL = new URL(docURL).href; const value = extractor.extractValue(spec); if (value === undefined || value === "") { - return result; + return defaultStartURL; } let potentialResult; try { @@ -269,17 +270,16 @@ var ManifestProcessor = { } catch (e) { const warn = domBundle.GetStringFromName("ManifestStartURLInvalid"); errors.push({ warn }); - return result; + return defaultStartURL; } if (potentialResult.origin !== docURL.origin) { const warn = domBundle.GetStringFromName( "ManifestStartURLShouldBeSameOrigin" ); errors.push({ warn }); - } else { - result = potentialResult.href; + return defaultStartURL; } - return result; + return potentialResult.href; } function processThemeColorMember() { @@ -314,6 +314,42 @@ var ManifestProcessor = { }; return extractor.extractLanguageValue(spec); } + + function processIdMember() { + // the start_url serves as the fallback, in case the id is not specified + // or in error. A start_url is assured. + const startURL = new URL(processedManifest.start_url); + + const spec = { + objectName: "manifest", + object: rawManifest, + property: "id", + expectedType: "string", + trim: false, + }; + const extractedValue = extractor.extractValue(spec); + + if (typeof extractedValue !== "string" || extractedValue === "") { + return startURL.href; + } + + let appId; + try { + appId = new URL(extractedValue, startURL.origin); + } catch { + const warn = domBundle.GetStringFromName("ManifestIdIsInvalid"); + errors.push({ warn }); + return startURL.href; + } + + if (appId.origin !== startURL.origin) { + const warn = domBundle.GetStringFromName("ManifestIdNotSameOrigin"); + errors.push({ warn }); + return startURL.href; + } + + return appId.href; + } }, }; var EXPORTED_SYMBOLS = ["ManifestProcessor"]; diff --git a/dom/manifest/test/mochitest.ini b/dom/manifest/test/mochitest.ini index c496caed42b80..73d7a6578707f 100644 --- a/dom/manifest/test/mochitest.ini +++ b/dom/manifest/test/mochitest.ini @@ -13,6 +13,7 @@ support-files = [test_ManifestProcessor_dir.html] [test_ManifestProcessor_display.html] [test_ManifestProcessor_icons.html] +[test_ManifestProcessor_id.html] [test_ManifestProcessor_JSON.html] [test_ManifestProcessor_lang.html] [test_ManifestProcessor_name_and_short_name.html] diff --git a/dom/manifest/test/test_ManifestProcessor_id.html b/dom/manifest/test/test_ManifestProcessor_id.html new file mode 100644 index 0000000000000..336d1d3a773bd --- /dev/null +++ b/dom/manifest/test/test_ManifestProcessor_id.html @@ -0,0 +1,123 @@ + + + + + + Test for Bug 1731940 - implement id member + + + + + diff --git a/dom/manifest/test/test_ManifestProcessor_start_url.html b/dom/manifest/test/test_ManifestProcessor_start_url.html index af9662d57d6a8..1d172df555497 100644 --- a/dom/manifest/test/test_ManifestProcessor_start_url.html +++ b/dom/manifest/test/test_ManifestProcessor_start_url.html @@ -40,9 +40,11 @@ result = processor.process(data); is(result.start_url.toString(), docURL.toString(), expected); - -// Resolve URLs relative to manfiest -var URLs = ["path", "/path", "../../path", +// Resolve URLs relative to manifest +var URLs = [ + "path", + "/path", + "../../path", `${whiteSpace}path${whiteSpace}`, `${whiteSpace}/path`, `${whiteSpace}../../path`, @@ -58,5 +60,24 @@ is(result.start_url.toString(), absURL, expected); }); - - +// It retains the fragment +var startURL = "./path?query=123#fragment"; +data.jsonText = JSON.stringify({ + start_url: startURL, +}); +var absURL = new URL(startURL, manifestURL).href; +result = processor.process(data); +is(result.start_url.toString(), absURL, "Retains fragment"); + +// It retains the fragment on the document's location too. +window.location = "#here"; +data.jsonText = JSON.stringify({}); +result = processor.process(data); +is( + window.location.href, + result.start_url.toString(), + `Retains the fragment of document's location` +); + + + diff --git a/dom/manifest/test/test_ManifestProcessor_warnings.html b/dom/manifest/test/test_ManifestProcessor_warnings.html index 04e0c2eddbd27..f2092c1dcf2f6 100644 --- a/dom/manifest/test/test_ManifestProcessor_warnings.html +++ b/dom/manifest/test/test_ManifestProcessor_warnings.html @@ -112,6 +112,25 @@ }, warn: "icons item at index 1 includes repeated purpose(s): any maskable.", }, + // testing dom.properties: ManifestIdIsInvalid + { + func() { + return (options.jsonText = JSON.stringify({ + id: "http://test:65536/foo", + })); + }, + warn: "The id member did not resolve to a valid URL.", + }, + // testing dom.properties ManifestIdNotSameOrigin + { + func() { + return (options.jsonText = JSON.stringify({ + id: "https://other.com", + start_url: "/this/place" + })); + }, + warn: "The id member must have the same origin as the start_url member.", + } ].forEach((test, index) => { test.func(); const result = processor.process(options);