Title: [228077] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (228076 => 228077)


--- branches/safari-605-branch/Source/WebCore/ChangeLog	2018-02-05 05:30:51 UTC (rev 228076)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog	2018-02-05 05:30:54 UTC (rev 228077)
@@ -1,5 +1,26 @@
 2018-02-04  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r228036. rdar://problem/37220130
+
+    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-04  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r228034. rdar://problem/37220140
 
     2018-02-02  Youenn Fablet  <you...@apple.com>

Modified: branches/safari-605-branch/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp (228076 => 228077)


--- branches/safari-605-branch/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp	2018-02-05 05:30:51 UTC (rev 228076)
+++ branches/safari-605-branch/Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp	2018-02-05 05:30:54 UTC (rev 228077)
@@ -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: branches/safari-605-branch/Tools/ChangeLog (228076 => 228077)


--- branches/safari-605-branch/Tools/ChangeLog	2018-02-05 05:30:51 UTC (rev 228076)
+++ branches/safari-605-branch/Tools/ChangeLog	2018-02-05 05:30:54 UTC (rev 228077)
@@ -1,5 +1,23 @@
 2018-02-04  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r228036. rdar://problem/37220130
+
+    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-04  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r228025. rdar://problem/37220140
 
     2018-02-02  Youenn Fablet  <you...@apple.com>

Modified: branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp (228076 => 228077)


--- branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp	2018-02-05 05:30:51 UTC (rev 228076)
+++ branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp	2018-02-05 05:30:54 UTC (rev 228077)
@@ -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" };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to