Commit 0b543dd3 authored by dominickn's avatar dominickn Committed by Commit bot

Properly fall back to manifest short_name when name is empty.

If a PWA includes a "name" field in their manifest, but explicitly sets
it to the empty string, it would have an app banner displayed with no
text, as well as no title on its splash screen.

This CL fixes the bug by properly checking if the "name" string is
empty, rather than just null. This check is already performed by the
InstallableManager in the process of determining whether or not a site
is a valid PWA. An additional test is added to ensure the correct
functionality.

BUG=722615

Review-Url: https://codereview.chromium.org/2900623002
Cr-Commit-Position: refs/heads/master@{#473806}
parent 9cc9427a
......@@ -97,6 +97,9 @@ public class AppBannerManagerTest {
private static final String WEB_APP_SHORT_TITLE_MANIFEST =
"/chrome/test/data/banners/manifest_short_name_only.json";
private static final String WEB_APP_EMPTY_NAME_MANIFEST =
"/chrome/test/data/banners/manifest_empty_name.json";
private static final String WEB_APP_TITLE = "Manifest test app";
private static final String WEB_APP_SHORT_TITLE = "Manifest";
......@@ -611,12 +614,21 @@ public class AppBannerManagerTest {
@Test
@SmallTest
@Feature({"AppBanners"})
public void testBannerFallsBackToShortName() throws Exception {
public void testBannerFallsBackToShortNameWhenNameNotPresent() throws Exception {
triggerWebAppBanner(WebappTestPage.urlOfPageWithServiceWorkerAndManifest(
mTestServer, WEB_APP_SHORT_TITLE_MANIFEST),
WEB_APP_SHORT_TITLE, false);
}
@Test
@SmallTest
@Feature({"AppBanners"})
public void testBannerFallsBackToShortNameWhenNameIsEmpty() throws Exception {
triggerWebAppBanner(WebappTestPage.urlOfPageWithServiceWorkerAndManifest(
mTestServer, WEB_APP_EMPTY_NAME_MANIFEST),
WEB_APP_SHORT_TITLE, false);
}
@Test
@SmallTest
@Feature({"AppBanners"})
......
......@@ -219,8 +219,13 @@ void AppBannerManager::OnDidGetManifest(const InstallableData& data) {
manifest_url_ = data.manifest_url;
manifest_ = data.manifest;
app_title_ = (manifest_.name.is_null()) ? manifest_.short_name.string()
: manifest_.name.string();
// One of manifest_.name or manifest_.short_name must be non-null and
// non-empty if the error code was NO_ERROR_DETECTED.
if (manifest_.name.is_null() || manifest_.name.string().empty())
app_title_ = manifest_.short_name.string();
else
app_title_ = manifest_.name.string();
PerformInstallableCheck();
}
......
{
"short_name": "Manifest",
"name": "",
"icons": [
{
"src": "launcher-icon-1x.png",
"sizes": "48x48",
"type": "image/png"
},
{
"src": "launcher-icon-1-5x.png",
"sizes": "72x72",
"type": "image/png"
},
{
"src": "launcher-icon-2x.png",
"sizes": "96x96",
"type": "image/png"
},
{
"src": "launcher-icon-3x.png",
"sizes": "144x144",
"type": "image/png"
},
{
"src": "launcher-icon-4x.png",
"sizes": "192x192",
"type": "image/png"
},
{
"src": "image-512px.png",
"sizes": "512x512",
"type": "image/png"
}
],
"start_url": "manifest_test_page.html",
"display": "standalone",
"orientation": "landscape"
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment