Modified: trunk/Source/WebCore/ChangeLog (228035 => 228036)
--- trunk/Source/WebCore/ChangeLog 2018-02-03 00:43:14 UTC (rev 228035)
+++ trunk/Source/WebCore/ChangeLog 2018-02-03 00:49:56 UTC (rev 228036)
@@ -1,3 +1,20 @@
+2018-02-02 David Quesada <david_ques...@apple.com>
+
+ WebAppManifest scope should default to the containing directory of start_url if 'scope' is not specified
+ https://bugs.webkit.org/show_bug.cgi?id=182363
+ rdar://problem/37093498
+
+ Reviewed by Ryosuke Niwa.
+
+ If an app manifest doesn't specify a scope, we should default to the "parent directory" of
+ the start URL, rather than leaving the app unbounded. This is more reasonable than using the
+ entire internet as the app scope.
+
+ No new tests, updates to the existing tests verify the new behavior.
+
+ * Modules/applicationmanifest/ApplicationManifestParser.cpp:
+ (WebCore::ApplicationManifestParser::parseScope):
+
2018-02-02 Youenn Fablet <you...@apple.com>
Clearing all service worker registrations should wait for importing service worker registration to finish
Modified: trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp (228035 => 228036)
--- trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp 2018-02-03 00:43:14 UTC (rev 228035)
+++ trunk/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp 2018-02-03 00:49:56 UTC (rev 228036)
@@ -193,23 +193,25 @@
URL ApplicationManifestParser::parseScope(const JSON::Object& manifest, const URL& documentURL, const URL& startURL)
{
+ URL defaultScope { startURL, "./" };
+
RefPtr<JSON::Value> value;
if (!manifest.getValue("scope", value))
- return { };
+ return defaultScope;
String stringValue;
if (!value->asString(stringValue)) {
logManifestPropertyNotAString(ASCIILiteral("scope"));
- return { };
+ return defaultScope;
}
if (stringValue.isEmpty())
- return { };
+ return defaultScope;
URL scopeURL(m_manifestURL, stringValue);
if (!scopeURL.isValid()) {
logManifestPropertyInvalidURL(ASCIILiteral("scope"));
- return { };
+ return defaultScope;
}
if (!protocolHostAndPortAreEqual(scopeURL, documentURL)) {
@@ -216,12 +218,12 @@
auto scopeURLOrigin = SecurityOrigin::create(scopeURL);
auto documentOrigin = SecurityOrigin::create(documentURL);
logDeveloperWarning(ASCIILiteral("The scope's origin of \"") + scopeURLOrigin->toString() + ASCIILiteral("\" is different from the document's origin of \"") + documentOrigin->toString() + ASCIILiteral("\"."));
- return { };
+ return defaultScope;
}
if (!isInScope(scopeURL, startURL)) {
logDeveloperWarning(ASCIILiteral("The start URL is not within scope of the provided scope URL."));
- return { };
+ return defaultScope;
}
return scopeURL;
Modified: trunk/Tools/ChangeLog (228035 => 228036)
--- trunk/Tools/ChangeLog 2018-02-03 00:43:14 UTC (rev 228035)
+++ trunk/Tools/ChangeLog 2018-02-03 00:49:56 UTC (rev 228036)
@@ -1,3 +1,17 @@
+2018-02-02 David Quesada <david_ques...@apple.com>
+
+ WebAppManifest scope should default to the containing directory of start_url if 'scope' is not specified
+ https://bugs.webkit.org/show_bug.cgi?id=182363
+ rdar://problem/37093498
+
+ Reviewed by Ryosuke Niwa.
+
+ Updated ApplicationManifestParserTests to account for the new default scope behavior.
+
+ * TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp:
+ (assertManifestHasDefaultValues):
+ (TEST_F):
+
2018-02-02 Youenn Fablet <you...@apple.com>
Clearing all service worker registrations should wait for importing service worker registration to finish
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp (228035 => 228036)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp 2018-02-03 00:43:14 UTC (rev 228035)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp 2018-02-03 00:49:56 UTC (rev 228036)
@@ -135,8 +135,8 @@
EXPECT_TRUE(manifest.name.isNull());
EXPECT_TRUE(manifest.shortName.isNull());
EXPECT_TRUE(manifest.description.isNull());
- EXPECT_TRUE(manifest.scope.isEmpty());
- EXPECT_STREQ(manifest.startURL.string().utf8().data(), documentURL.string().utf8().data());
+ EXPECT_STREQ("https://example.com/", manifest.scope.string().utf8().data());
+ EXPECT_STREQ(documentURL.string().utf8().data(), manifest.startURL.string().utf8().data());
}
TEST_F(ApplicationManifestParserTest, DefaultManifest)
@@ -247,27 +247,32 @@
TEST_F(ApplicationManifestParserTest, Scope)
{
- testScope("123", String());
- testScope("null", String());
- testScope("true", String());
- testScope("{ }", String());
- testScope("[ ]", String());
- testScope("\"\"", String());
+ // If the scope is not a string or not a valid URL, return the default scope (the parent path of the start URL).
+ m_documentURL = { { }, "https://example.com/a/page?queryParam=value#fragment" };
+ m_manifestURL = { { }, "https://example.com/manifest.json" };
+ testScope("123", "https://example.com/a/");
+ testScope("null", "https://example.com/a/");
+ testScope("true", "https://example.com/a/");
+ testScope("{ }", "https://example.com/a/");
+ testScope("[ ]", "https://example.com/a/");
+ testScope("\"\"", "https://example.com/a/");
+ testScope("\"http:?\"", "https://example.com/a/");
- testScope("\"http:?\"", String());
+ m_documentURL = { { }, "https://example.com/a/pageEndingWithSlash/" };
+ testScope("null", "https://example.com/a/pageEndingWithSlash/");
- // If scope URL is not same origin as document URL, return undefined.
+ // If scope URL is not same origin as document URL, return the default scope.
m_documentURL = { { }, "https://example.com/home" };
m_manifestURL = { { }, "https://other-site.com/manifest.json" };
- testScope("\"https://other-site.com/some-scope\"", String());
+ testScope("\"https://other-site.com/some-scope\"", "https://example.com/");
m_documentURL = { { }, "https://example.com/app/home" };
m_manifestURL = { { }, "https://example.com/app/manifest.json" };
- // If start URL is not within scope of scope URL, return undefined
- testScope("\"https://example.com/subdirectory\"", String());
+ // If start URL is not within scope of scope URL, return the default scope.
+ testScope("\"https://example.com/subdirectory\"", "https://example.com/app/");
testScope("\"https://example.com/app\"", "https://example.com/app");
- testScope("\"https://example.com/APP\"", String());
+ testScope("\"https://example.com/APP\"", "https://example.com/app/");
testScope("\"https://example.com/a\"", "https://example.com/a");
m_documentURL = { { }, "https://example.com/a/b/c/index" };
