10BC0 Bug 1731940 - Web Manifest: implement id processing r=saschanaz · jwidar/LatencyZeroGithub@0b15ed6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0b15ed6

Browse files
marcoscaceresmarcoscaceres
committed
Bug 1731940 - Web Manifest: implement id processing r=saschanaz
spec change w3c/manifest#988 Differential Revision: https://phabricator.services.mozilla.com/D126331
1 parent 204500b commit 0b15ed6

File tree

6 files changed

+213
-11
lines changed

6 files changed

+213
-11
lines changed

‎dom/locales/en-US/chrome/dom/dom.properties‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ ServiceWorkerGraceTimeoutTermination=Terminating ServiceWorker for scope ‘%1$S
212212
# LOCALIZATION NOTE (ServiceWorkerNoFetchHandler): Do not translate "Fetch".
213213
ServiceWorkerNoFetchHandler=Fetch event handlers must be added during the worker script’s initial evaluation.
214214
ExecCommandCutCopyDeniedNotInputDriven=document.execCommand(‘cut’/‘copy’) was denied because it was not called from inside a short running user-generated event handler.
215+
ManifestIdIsInvalid=The id member did not resolve to a valid URL.
216+
ManifestIdNotSameOrigin=The id member must have the same origin as the start_url member.
215217
ManifestShouldBeObject=Manifest should be an object.
216218
ManifestScopeURLInvalid=The scope URL is invalid.
217219
ManifestScopeNotSameOrigin=The scope URL must be same origin as document.

‎dom/manifest/ManifestProcessor.jsm‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ var ManifestProcessor = {
124124
background_color: processBackgroundColorMember(),
125125
};
126126
processedManifest.scope = processScopeMember();
127+
processedManifest.id = processIdMember();
127128
if (checkConformance) {
128129
processedManifest.moz_validation = errors;
129130
processedManifest.moz_manifest_url = manifestURL.href;
@@ -258,28 +259,27 @@ var ManifestProcessor = {
258259
expectedType: "string",
259260
trim: false,
260261
};
261-
let result = new URL(docURL).href;
262+
const defaultStartURL = new URL(docURL).href;
262263
const value = extractor.extractValue(spec);
263264
if (value === undefined || value === "") {
264-
return result;
265+
return defaultStartURL;
265266
}
266267
let potentialResult;
267268
try {
268269
potentialResult = new URL(value, manifestURL);
269270
} catch (e) {
270271
const warn = domBundle.GetStringFromName("ManifestStartURLInvalid");
271272
errors.push({ warn });
272-
return result;
273+
return defaultStartURL;
273274
}
274275
if (potentialResult.origin !== docURL.origin) {
275276
const warn = domBundle.GetStringFromName(
276277
"ManifestStartURLShouldBeSameOrigin"
277278
);
278279
errors.push({ warn });
279-
} else {
280-
result = potentialResult.href;
280+
return defaultStartURL;
281281
}
282-
return result;
282+
return potentialResult.href;
283283
}
284284

285285
function processThemeColorMember() {
@@ -314,6 +314,42 @@ var ManifestProcessor = {
314314
};
315315
return extractor.extractLanguageValue(spec);
316316
}
317+
318+
function processIdMember() {
319+
// the start_url serves as the fallback, in case the id is not specified
320+
// or in error. A start_url is assured.
321+
const startURL = new URL(processedManifest.start_url);
322+
323+
const spec = {
324+
objectName: "manifest",
325+
object: rawManifest,
326+
property: "id",
327+
expectedType: "string",
328+
trim: false,
329+
};
330+
const extractedValue = extractor.extractValue(spec);
331+
332+
if (typeof extractedValue !== "string" || extractedValue === "") {
333+
return startURL.href;
334+
}
335+
336+
let appId;
337+
try {
338+
appId = new URL(extractedValue, startURL.origin);
339+
} catch {
340+
const warn = domBundle.GetStringFromName("ManifestIdIsInvalid");
341+
errors.push({ warn });
342+
return startURL.href;
343+
}
344+
345+
if (appId.origin !== startURL.origin) {
346+
const warn = domBundle.GetStringFromName("ManifestIdNotSameOrigin");
347+
errors.push({ warn });
348+
return startURL.href;
349+
}
350+
351+
return appId.href;
352+
}
317353
},
318354
};
319355
var EXPORTED_SYMBOLS = ["ManifestProcessor"];

‎dom/manifest/test/mochitest.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ support-files =
1313
[test_ManifestProcessor_dir.html]
1414
[test_ManifestProcessor_display.html]
1515
[test_ManifestProcessor_icons.html]
16+
[test_ManifestProcessor_id.html]
1617
[test_ManifestProcessor_JSON.html]
1718
[test_ManifestProcessor_lang.html]
1819
[test_ManifestProcessor_name_and_short_name.html]

‎dom/manifest/test/test_ManifestProcessor_id.html‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<!DOCTYPE HTML>
2+
<html>
3+
<!--
4+
https://bugzilla.mozilla.org/show_bug.cgi?id=1731940
5+
-->
6+
<head>
7+
<meta charset="utf-8">
8+
<title>Test for Bug 1731940 - implement id member</title>
9+
<script src="/tests/SimpleTest/SimpleTest.js"></script>
10+
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
11+
<script src="common.js"></script>
12+
<script>
13+
/**
14+
* Manifest id member
15+
* https://w3c.github.io/manifest/#id-member
16+
**/
17+
for (const type of typeTests) {
18+
data.jsonText = JSON.stringify({
19+
id: type,
20+
});
21+
const result = processor.process(data);
22+
is(
23+
result.id.toString(),
24+
result.start_url.toString(),
25+
`Expect non-string id to fall back to start_url: ${typeof type}.`
26+
);
27+
}
28+
29+
// Invalid URLs
30+
const invalidURLs = [
31+
"https://foo:65536",
32+
"https://foo\u0000/",
33+
"//invalid:65555",
34+
"file:///passwords",
35+
"about:blank",
36+
"data:text/html,<html><script>alert('lol')<\/script></html>",
37+
];
38+
39+
for (const url of invalidURLs) {
40+
data.jsonText = JSON.stringify({
41+
id: url,
42+
});
43+
const result = processor.process(data);
44+
is(
45+
result.id.toString(),
46+
result.start_url.toString(),
47+
"Expect invalid id URL to fall back to start_url."
48+
);
49+
}
50+
51+
// Not same origin
52+
data.jsonText = JSON.stringify({
53+
id: "http://not-same-origin",
54+
});
55+
var result = processor.process(data);
56+
is(
57+
result.id.toString(),
58+
result.start_url,
59+
"Expect different origin id to fall back to start_url."
60+
);
61+
62+
// Empty string test
63+
data.jsonText = JSON.stringify({
64+
id: "",
65+
});
66+
result = processor.process(data);
67+
is(
68+
result.id.toString(),
69+
result.start_url.toString(),
70+
`Expect empty string for id to use start_url.`
71+
);
72+
73+
// Resolve URLs relative to the start_url's origin
74+
const URLs = [
75+
"path",
76+
"/path",
77+
"../../path",
78+
"./path",
79+
`${whiteSpace}path${whiteSpace}`,
80+
`${whiteSpace}/path`,
81+
`${whiteSpace}../../path`,
82+
`${whiteSpace}./path`,
83+
];
84+
85+
for (const url of URLs) {
86+
data.jsonText = JSON.stringify({
87+
id: url,
88+
start_url: "/path/some.html",
89+
});
90+
result = processor.process(data);
91+
const baseOrigin = new URL(result.start_url.toString()).origin;
92+
const expectedUrl = new URL(url, baseOrigin).toString();
93+
is(
94+
result.id.toString(),
95+
expectedUrl,
96+
"Expected id to be resolved relative to start_url's origin."
97+
);
98+
}
99+
100+
// Handles unicode encoded URLs
101+
const specialCases = [
102+
["😀", "%F0%9F%98%80"],
103+
[
104+
"this/is/ok?query_is_ok=😀#keep_hash",
105+
"this/is/ok?query_is_ok=%F0%9F%98%80#keep_hash",
106+
],
107+
];
108+
for (const [id, expected] of specialCases) {
109+
data.jsonText = JSON.stringify({
110+
id,
111+
start_url: "/my-app/",
112+
});
113+
result = processor.process(data);
114+
const baseOrigin = new URL(result.start_url.toString()).origin;
115+
const expectedUrl = new URL(expected, baseOrigin).toString();
116+
is(
117+
result.id.toString(),
118+
expectedUrl,
119+
`Expect id to be encoded/decoded per URL spec.`
120+
);
121+
}
122+
</script>
123+
</head>

‎dom/manifest/test/test_ManifestProcessor_start_url.html‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@
4040
result = processor.process(data);
4141
is(result.start_url.toString(), docURL.toString(), expected);
4242

43-
44-
// Resolve URLs relative to manfiest
45-
var URLs = ["path", "/path", "../../path",
43+
// Resolve URLs relative to manifest
44+
var URLs = [
45+
"path",
46+
"/path",
47+
"../../path",
4648
`${whiteSpace}path${whiteSpace}`,
4749
`${whiteSpace}/path`,
4850
`${whiteSpace}../../path`,
@@ -58,5 +60,24 @@
5860
is(result.start_url.toString(), absURL, expected);
5961
});
6062

61-
</script>
62-
</head>
63+
// It retains the fragment
64+
var startURL = "./path?query=123#fragment";
65+
data.jsonText = JSON.stringify({
66+
start_url: startURL,
67+
});
68+
var absURL = new URL(startURL, manifestURL).href;
69+
result = processor.process(data);
70+
is(result.start_url.toString(), absURL, "Retains fragment");
71+
72+
// It retains the fragment on the document's location too.
73+
window.location = "#here";
74+
data.jsonText = JSON.stringify({});
75+
result = processor.process(data);
76+
is(
77+
window.location.href,
78+
result.start_url.toString(),
79+
`Retains the fragment of document's location`
80+
);
81+
</script>
82+
</head>
83+
</html>

‎dom/manifest/test/test_ManifestProcessor_warnings.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,25 @@
112112
},
113113
warn: "icons item at index 1 includes repeated purpose(s): any maskable.",
114114
},
115+
// testing dom.properties: ManifestIdIsInvalid
116+
{
117+
func() {
118+
return (options.jsonText = JSON.stringify({
119+
id: "http://test:65536/foo",
120+
}));
121+
},
122+
warn: "The id member did not resolve to a valid URL.",
123+
},
124+
// testing dom.properties ManifestIdNotSameOrigin
125+
{
126+
func() {
127+
return (options.jsonText = JSON.stringify({
128+
id: "https://other.com",
129+
start_url: "/this/place"
130+
}));
131+
},
132+
warn: "The id member must have the same origin as the start_url member.",
133+
}
115134
].forEach((test, index) => {
116135
test.func();
117136
const result = processor.process(options);

0 commit comments

Comments
 (0)
0