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.',