Parse the 'translations' manifest field.

This CL adds parsing for the translations field proposed in: w3c/manifest#676. It is behind the flag blink::features::kWebAppEnableTranslations which is disabled by default. Bug: 1259777 Change-Id: I9e64851fa53fa633a47a4ef5bdbf43f2a50060f2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3220767 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Commit-Queue: Louise Brett <loubrett@google.com> Cr-Commit-Position: refs/heads/main@{#933341} NOKEYCHECK=True GitOrigin-RevId: 3b01317fd0f6373831d1bb9e8ba334b24d919462
mjfroman · Oct 20, 2021 · c95496f · c95496f
1 parent 3548686
commit c95496f
Show file tree

Hide file tree

Showing 7 changed files with 241 additions and 0 deletions.
diff --git a/blink/common/features.cc b/blink/common/features.cc
@@ -819,6 +819,10 @@ const base::Feature kWebAppEnableLinkCapturing{
 const base::Feature kWebAppEnableManifestId{"WebAppEnableManifestId",
                                             base::FEATURE_ENABLED_BY_DEFAULT};
 
+// Enables the "translations" manifest field for web apps.
+const base::Feature kWebAppEnableTranslations{
+    "WebAppEnableTranslations", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Controls URL handling feature in web apps. Controls parsing of "url_handlers"
 // field in web app manifests. See explainer for more information:
 // https://github.com/WICG/pwa-url-handler/blob/master/explainer.md

diff --git a/blink/public/common/features.h b/blink/public/common/features.h
@@ -329,6 +329,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kWebAppEnableLinkCapturing;
 
 BLINK_COMMON_EXPORT extern const base::Feature kWebAppEnableManifestId;
 
+BLINK_COMMON_EXPORT extern const base::Feature kWebAppEnableTranslations;
+
 BLINK_COMMON_EXPORT extern const base::Feature kWebAppEnableUrlHandlers;
 
 BLINK_COMMON_EXPORT extern const base::Feature kWebAppEnableProtocolHandlers;

diff --git a/blink/public/mojom/manifest/manifest.mojom b/blink/public/mojom/manifest/manifest.mojom
@@ -103,6 +103,11 @@ struct Manifest {
   // experiment. See:
   // https://github.com/WICG/sw-launch/blob/main/launch_handler.md
   ManifestLaunchHandler? launch_handler;
+
+  // TODO(crbug.com/1259777): This field is non-standard and part of a Chrome
+  // experiment. See: https://github.com/w3c/manifest/issues/676#issuecomment-810628993
+  // A mapping of locales to their translations.
+  map<mojo_base.mojom.String16, ManifestTranslationItem> translations;
 };
 
 // Structure representing a Shortcut Item per the Manifest specification, see:
@@ -272,6 +277,18 @@ struct ManifestLaunchHandler {
   NavigateExistingClient navigate_existing_client;
 };
 
+// Structure representing the translated manifest fields. Each field corresponds
+// to a translatable field in the Web App Manifest and contains a translation of
+// that field. Each field is optional as there is no mandatory field which must
+// be translated in any locale.
+struct ManifestTranslationItem {
+  mojo_base.mojom.String16? name;
+
+  mojo_base.mojom.String16? short_name;
+
+  mojo_base.mojom.String16? description;
+};
+
 // Debug information for a parsed manifest.
 struct ManifestDebugInfo {
   array<ManifestError> errors;

diff --git a/blink/renderer/modules/manifest/manifest_parser.cc b/blink/renderer/modules/manifest/manifest_parser.cc
@@ -167,6 +167,10 @@ void ManifestParser::Parse() {
 
   manifest_->launch_handler = ParseLaunchHandler(root_object.get());
 
+  if (RuntimeEnabledFeatures::WebAppTranslationsEnabled(feature_context_)) {
+    manifest_->translations = ParseTranslations(root_object.get());
+  }
+
   ManifestUmaUtil::ParseSucceeded(manifest_);
 }
 
@@ -1483,6 +1487,63 @@ mojom::blink::ManifestLaunchHandlerPtr ManifestParser::ParseLaunchHandler(
                                                   navigate_existing_client);
 }
 
+HashMap<String, mojom::blink::ManifestTranslationItemPtr>
+ManifestParser::ParseTranslations(const JSONObject* object) {
+  HashMap<String, mojom::blink::ManifestTranslationItemPtr> result;
+
+  if (!object->Get("translations"))
+    return result;
+
+  JSONObject* translations_map = object->GetJSONObject("translations");
+  if (!translations_map) {
+    AddErrorInfo("property 'translations' ignored, object expected.");
+    return result;
+  }
+
+  for (wtf_size_t i = 0; i < translations_map->size(); ++i) {
+    JSONObject::Entry entry = translations_map->at(i);
+    String locale = entry.first;
+    if (locale == "") {
+      AddErrorInfo("skipping translation, non-empty locale string expected.");
+      continue;
+    }
+    JSONObject* translation = JSONObject::Cast(entry.second);
+    if (!translation) {
+      AddErrorInfo("skipping translation, object expected.");
+      continue;
+    }
+
+    auto translation_item = mojom::blink::ManifestTranslationItem::New();
+
+    absl::optional<String> name =
+        ParseStringForMember(translation, "translations", "name", false, Trim);
+    translation_item->name =
+        name.has_value() && name->length() != 0 ? *name : String();
+
+    absl::optional<String> short_name = ParseStringForMember(
+        translation, "translations", "short_name", false, Trim);
+    translation_item->short_name =
+        short_name.has_value() && short_name->length() != 0 ? *short_name
+                                                            : String();
+
+    absl::optional<String> description = ParseStringForMember(
+        translation, "translations", "description", false, Trim);
+    translation_item->description =
+        description.has_value() && description->length() != 0 ? *description
+                                                              : String();
+
+    // A translation may be specified for any combination of translatable fields
+    // in the manifest. If no translations are supplied, we skip this item.
+    if (!translation_item->name && !translation_item->short_name &&
+        !translation_item->description) {
+      continue;
+    }
+
+    result.Set(locale, std::move(translation_item));
+  }
+  return result;
+}
+
 void ManifestParser::AddErrorInfo(const String& error_msg,
                                   bool critical,
                                   int error_line,

diff --git a/blink/renderer/modules/manifest/manifest_parser.h b/blink/renderer/modules/manifest/manifest_parser.h
@@ -436,6 +436,12 @@ class MODULES_EXPORT ManifestParser {
   mojom::blink::ManifestLaunchHandlerPtr ParseLaunchHandler(
       const JSONObject* object);
 
+  // Parses the 'translations' field of the manifest as defined in:
+  // https://github.com/w3c/manifest/issues/676#issuecomment-810628993
+  // Returns empty map if parsing fails.
+  HashMap<String, mojom::blink::ManifestTranslationItemPtr> ParseTranslations(
+      const JSONObject* object);
+
   void AddErrorInfo(const String& error_msg,
                     bool critical = false,
                     int error_line = 0,

diff --git a/blink/renderer/modules/manifest/manifest_parser_unittest.cc b/blink/renderer/modules/manifest/manifest_parser_unittest.cc
@@ -4958,4 +4958,151 @@ TEST_F(ManifestParserTest, LaunchHandlerParseRules) {
   }
 }
 
+TEST_F(ManifestParserTest, TranslationsParseRules) {
+  {
+    ScopedWebAppTranslationsForTest feature(false);
+
+    // Feature not enabled, should not be parsed.
+    auto& manifest =
+        ParseManifest(R"({ "translations": {"fr": {"name": "french name"}} })");
+    EXPECT_TRUE(manifest->translations.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+  {
+    ScopedWebAppTranslationsForTest feature(true);
+
+    // Manifest does not contain a 'translations' field.
+    {
+      auto& manifest = ParseManifest(R"({ })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Don't parse if translations object is empty.
+    {
+      auto& manifest = ParseManifest(R"({ "translations": {} })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Empty translation is ignored.
+    {
+      auto& manifest = ParseManifest(R"({ "translations": {"fr": {}} })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_FALSE(manifest->translations.Contains("fr"));
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Valid name, short_name and description should be parsed
+    {
+      auto& manifest = ParseManifest(
+          R"({ "translations": {"fr": {"name": "french name", "short_name":
+           "fr name", "description": "french description"}} })");
+      EXPECT_FALSE(manifest->translations.IsEmpty());
+      EXPECT_TRUE(manifest->translations.Contains("fr"));
+      EXPECT_EQ(manifest->translations.find("fr")->value->name, "french name");
+      EXPECT_EQ(manifest->translations.find("fr")->value->short_name,
+                "fr name");
+      EXPECT_EQ(manifest->translations.find("fr")->value->description,
+                "french description");
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Don't parse if the property isn't an object.
+    {
+      auto& manifest = ParseManifest(R"({ "translations": [] })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("property 'translations' ignored, object expected.",
+                errors()[0]);
+    }
+
+    // Ignore translation if it isn't an object.
+    {
+      auto& manifest = ParseManifest(R"({ "translations": {"fr": []} })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("skipping translation, object expected.", errors()[0]);
+    }
+
+    // Multiple valid translations should all be parsed.
+    {
+      auto& manifest = ParseManifest(
+          R"({ "translations": {"fr": {"name": "french name"},
+          "es": {"name": "spanish name"}} })");
+      EXPECT_FALSE(manifest->translations.IsEmpty());
+      EXPECT_TRUE(manifest->translations.Contains("fr"));
+      EXPECT_TRUE(manifest->translations.Contains("es"));
+      EXPECT_EQ(manifest->translations.find("fr")->value->name, "french name");
+      EXPECT_EQ(manifest->translations.find("es")->value->name, "spanish name");
+      EXPECT_EQ(0u, GetErrorCount());
+    }
+
+    // Empty locale string should be ignored.
+    {
+      auto& manifest = ParseManifest(
+          R"({ "translations": {"": {"name": "translated name"}} })");
+      EXPECT_TRUE(manifest->translations.IsEmpty());
+      EXPECT_EQ(1u, GetErrorCount());
+      EXPECT_EQ("skipping translation, non-empty locale string expected.",
+                errors()[0]);
+    }
+  }
+}
+
+TEST_F(ManifestParserTest, TranslationsStringsParseRules) {
+  ScopedWebAppTranslationsForTest feature(true);
+
+  // Ignore non-string translations name.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "translations": {"fr": {"name": {}}} })");
+    EXPECT_TRUE(manifest->translations.IsEmpty());
+    EXPECT_EQ(1u, GetErrorCount());
+    EXPECT_EQ(
+        "property 'name' of 'translations' ignored, type string expected.",
+        errors()[0]);
+  }
+
+  // Ignore non-string translations short_name.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "translations": {"fr": {"short_name": []}} })");
+    EXPECT_TRUE(manifest->translations.IsEmpty());
+    EXPECT_EQ(1u, GetErrorCount());
+    EXPECT_EQ(
+        "property 'short_name' of 'translations' ignored, type string "
+        "expected.",
+        errors()[0]);
+  }
+
+  // Ignore non-string translations description.
+  {
+    auto& manifest =
+        ParseManifest(R"({ "translations": {"fr": {"description": 42}} })");
+    EXPECT_TRUE(manifest->translations.IsEmpty());
+    EXPECT_EQ(1u, GetErrorCount());
+    EXPECT_EQ(
+        "property 'description' of 'translations' ignored, type string "
+        "expected.",
+        errors()[0]);
+  }
+
+  // Translation with empty strings is ignored.
+  {
+    auto& manifest = ParseManifest(
+        R"({ "translations": {"fr": {"name": "", "short_name": "",
+        "description": ""}} })");
+    EXPECT_TRUE(manifest->translations.IsEmpty());
+    EXPECT_FALSE(manifest->translations.Contains("fr"));
+    EXPECT_EQ(3u, GetErrorCount());
+    EXPECT_EQ("property 'name' of 'translations' is an empty string.",
+              errors()[0]);
+    EXPECT_EQ("property 'short_name' of 'translations' is an empty string.",
+              errors()[1]);
+    EXPECT_EQ("property 'description' of 'translations' is an empty string.",
+              errors()[2]);
+  }
+}
+
 }  // namespace blink
diff --git a/blink/renderer/platform/runtime_enabled_features.json5 b/blink/renderer/platform/runtime_enabled_features.json5
@@ -2393,6 +2393,10 @@
       name: "WebAppTabStrip",
       status: "experimental",
     },
+    {
+      name: "WebAppTranslations",
+      status: "experimental",
+    },
     {
       // This flag enables the Manifest parser to handle URL Handlers.
       // Also enabled when blink::features::kWebAppEnableUrlHandlers is