[go: nahoru, domu]

DevTools: Validate web app manifest screenshots aspect ratio

This CL adds a warning if not all web app manifest screenshots matching
the appropriate form factor have the same aspect ratio.

Bug: 1476656
Change-Id: Ib8f55385dbd93ea1e1dde4cf8cf217ad0f9b38ae
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4818907
Reviewed-by: Wolfgang Beyer <wolfi@chromium.org>
Commit-Queue: Wolfgang Beyer <wolfi@chromium.org>
diff --git a/front_end/panels/application/AppManifestView.ts b/front_end/panels/application/AppManifestView.ts
index b4b9071..e701359 100644
--- a/front_end/panels/application/AppManifestView.ts
+++ b/front_end/panels/application/AppManifestView.ts
@@ -414,6 +414,11 @@
    */
   tooManyScreenshotsForMobile: 'No more than 5 screenshots will be displayed on mobile. The rest will be ignored.',
   /**
+   *@description Warning text about not all screenshots matching the appropriate form factor have the same aspect ratio
+   */
+  screenshotsMustHaveSameAspectRatio:
+      'All screenshots with the same "form_factor" must have the same aspect ratio as the first screenshot with that "form_factor". Some screenshots will be ignored.',
+  /**
    *@description Message for Window Controls Overlay value succsessfully found with links to documnetation
    *@example {window-controls-overlay} PH1
    *@example {https://developer.mozilla.org/en-US/docs/Web/Manifest/display_override} PH2
@@ -876,6 +881,8 @@
     }
 
     let screenshotIndex = 1;
+    const formFactorScreenshotDimensions = new Map<string, {width: number, height: number}>();
+    let haveScreenshotsDifferentAspectRatio = false;
     for (const screenshot of screenshots) {
       const screenshotSection =
           this.reportView.appendSection(i18nString(UIStrings.screenshotS, {PH1: screenshotIndex}));
@@ -891,12 +898,28 @@
         screenshotSection.appendFlexedField(i18nString(UIStrings.platform), screenshot.platform);
       }
 
-      const {imageResourceErrors: screenshotErrors} =
+      const {imageResourceErrors: screenshotErrors, naturalWidth: width, naturalHeight: height} =
           await this.appendImageResourceToSection(url, screenshot, screenshotSection, /** isScreenshot= */ true);
       imageErrors.push(...screenshotErrors);
+
+      if (screenshot.form_factor && width && height) {
+        formFactorScreenshotDimensions.has(screenshot.form_factor) ||
+            formFactorScreenshotDimensions.set(screenshot.form_factor, {width, height});
+        const formFactorFirstScreenshotDimensions = formFactorScreenshotDimensions.get(screenshot.form_factor);
+        if (formFactorFirstScreenshotDimensions) {
+          haveScreenshotsDifferentAspectRatio = haveScreenshotsDifferentAspectRatio ||
+              (width * formFactorFirstScreenshotDimensions.height !==
+               height * formFactorFirstScreenshotDimensions.width);
+        }
+      }
+
       screenshotIndex++;
     }
 
+    if (haveScreenshotsDifferentAspectRatio) {
+      warnings.push(i18nString(UIStrings.screenshotsMustHaveSameAspectRatio));
+    }
+
     const screenshotsForDesktop = screenshots.filter(screenshot => screenshot.form_factor === 'wide');
     const screenshotsForMobile = screenshots.filter(screenshot => screenshot.form_factor !== 'wide');
 
@@ -1176,8 +1199,12 @@
       // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration)
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       baseUrl: Platform.DevToolsPath.UrlString, imageResource: any, section: UI.ReportView.Section,
-      isScreenshot: boolean):
-      Promise<{imageResourceErrors: Platform.UIString.LocalizedString[], squareSizedIconAvailable?: boolean}> {
+      isScreenshot: boolean): Promise<{
+    imageResourceErrors: Platform.UIString.LocalizedString[],
+    squareSizedIconAvailable?: boolean,
+    naturalWidth?: number,
+    naturalHeight?: number,
+  }> {
     const imageResourceErrors: Platform.UIString.LocalizedString[] = [];
     const resourceName = isScreenshot ? i18nString(UIStrings.screenshot) : i18nString(UIStrings.icon);
     if (!imageResource.src) {
@@ -1196,6 +1223,7 @@
       return {imageResourceErrors};
     }
     const {wrapper, image} = result;
+    const {naturalWidth, naturalHeight} = image;
     const sizes = this.parseSizes(imageResource['sizes'], resourceName, imageUrl, imageResourceErrors);
     const title = sizes.map(x => x.formatted).join(' ') + '\n' + (imageResource['type'] || '');
     const field = section.appendFlexedField(title);
@@ -1239,7 +1267,7 @@
     }
 
     field.appendChild(wrapper);
-    return {imageResourceErrors, squareSizedIconAvailable};
+    return {imageResourceErrors, squareSizedIconAvailable, naturalWidth, naturalHeight};
   }
   override wasShown(): void {
     super.wasShown();
diff --git a/test/unittests/fixtures/images/640x320.png b/test/unittests/fixtures/images/640x320.png
new file mode 100644
index 0000000..a3bd8b9
--- /dev/null
+++ b/test/unittests/fixtures/images/640x320.png
Binary files differ
diff --git a/test/unittests/fixtures/images/BUILD.gn b/test/unittests/fixtures/images/BUILD.gn
index b7c25fb..393fde4 100644
--- a/test/unittests/fixtures/images/BUILD.gn
+++ b/test/unittests/fixtures/images/BUILD.gn
@@ -7,6 +7,7 @@
 copy_to_gen("images") {
   sources = [
     "320x320.png",
+    "640x320.png",
     "96x96.png",
   ]
 }
diff --git a/test/unittests/front_end/panels/application/AppManifestView_test.ts b/test/unittests/front_end/panels/application/AppManifestView_test.ts
index 310d2ee..825cb18 100644
--- a/test/unittests/front_end/panels/application/AppManifestView_test.ts
+++ b/test/unittests/front_end/panels/application/AppManifestView_test.ts
@@ -217,7 +217,7 @@
       assert.deepStrictEqual(actual, expected);
     });
 
-    it('displays warnings for too many desktop screenshots', async () => {
+    it('displays warnings for too many desktop screenshots and wrong aspect ratio', async () => {
       const actual = await renderWithWarnings(`{
         "screenshots": [
           {
@@ -227,9 +227,9 @@
             "form_factor": "wide"
           },
           {
-            "src": "/fixtures/images/320x320.png",
+            "src": "/fixtures/images/640x320.png",
             "type": "image/png",
-            "sizes": "320x320",
+            "sizes": "640x320",
             "form_factor": "wide"
           },
           {
@@ -277,6 +277,7 @@
         ]
       }`);
       const expected = [
+        'All screenshots with the same "form_factor" must have the same aspect ratio as the first screenshot with that "form_factor". Some screenshots will be ignored.',
         'Richer PWA Install UI won’t be available on mobile. Please add at least one screenshot for which "form_factor" is not set or set to a value other than "wide".',
         'No more than 8 screenshots will be displayed on desktop. The rest will be ignored.',
         'Most operating systems require square icons. Please include at least one square icon in the array.',