From e72b4191ea5edaa04749b7ecac5807a90b142e6c Mon Sep 17 00:00:00 2001
From: Kagami Sascha Rosylight <saschanaz@outlook.com>
Date: Sat, 11 Mar 2023 22:10:55 +0100
Subject: [PATCH] nullable width

---
 src/general.ts                                | 26 ++++++++++++-------
 test/index.ts                                 |  7 +++++
 test/oembed/invalid/oembed-invalid-width.json |  7 -----
 ...idth.json => oembed-percentage-width.json} |  1 +
 4 files changed, 25 insertions(+), 16 deletions(-)
 delete mode 100644 test/oembed/invalid/oembed-invalid-width.json
 rename test/oembed/{invalid/oembed-no-width.json => oembed-percentage-width.json} (85%)

diff --git a/src/general.ts b/src/general.ts
index b93bed4..d335e7a 100644
--- a/src/general.ts
+++ b/src/general.ts
@@ -64,13 +64,21 @@ async function getOEmbedPlayer($: cheerio.CheerioAPI, pageUrl: string): Promise<
 		return null;
 	}
 
-	if (
-		typeof body.width !== 'number' ||
-		body.width <= 0 ||
-		typeof body.height !== 'number' ||
-		body.height <= 0
-	) {
-		// No proper size info
+	// Height is the most important, width is okay to be null. The implementer
+	// should choose fixed height instead of fixed aspect ratio if width is null.
+	//
+	// For example, Spotify's embed page does not strictly follow aspect ratio
+	// and thus keeping the height is better than keeping the aspect ratio.
+	//
+	// Spotify gives `width: 100%, height: 152px` for iframe while `width: 456,
+	// height: 152` for oEmbed data, and we treat any percentages as null here.
+	let width: number | null = Number(iframe.attr('width') ?? body.width);
+	if (Number.isNaN(width)) {
+		width = null;
+	}
+	const height = Math.min(Number(iframe.attr('height') ?? body.height), 1024);
+	if (Number.isNaN(height)) {
+		// No proper height info
 		return null;
 	}
 
@@ -91,8 +99,8 @@ async function getOEmbedPlayer($: cheerio.CheerioAPI, pageUrl: string): Promise<
 
 	return {
 		url,
-		width: body.width,
-		height: body.height,
+		width,
+		height,
 		allow: allowedFeatures
 	}
 }
diff --git a/test/index.ts b/test/index.ts
index a8e7fb4..e1898ea 100644
--- a/test/index.ts
+++ b/test/index.ts
@@ -332,4 +332,11 @@ describe("oEmbed", () => {
 		expect(summary.player.url).toBe('https://example.com/');
 		expect(summary.player.allow).toStrictEqual([]);
 	});
+
+	test('width: 100%', async () => {
+		await setUpFastify('oembed-percentage-width.json');
+		const summary = await summaly(host);
+		expect(summary.player.width).toBe(null);
+		expect(summary.player.height).toBe(300);
+	});
 });
diff --git a/test/oembed/invalid/oembed-invalid-width.json b/test/oembed/invalid/oembed-invalid-width.json
deleted file mode 100644
index ea2673c..0000000
--- a/test/oembed/invalid/oembed-invalid-width.json
+++ /dev/null
@@ -1,7 +0,0 @@
-{
-  "version": "1.0",
-  "type": "rich",
-  "html": "<iframe src='https://example.com/'></iframe>",
-  "width": "blobcat",
-  "height": 300
-}
diff --git a/test/oembed/invalid/oembed-no-width.json b/test/oembed/oembed-percentage-width.json
similarity index 85%
rename from test/oembed/invalid/oembed-no-width.json
rename to test/oembed/oembed-percentage-width.json
index cd48cc3..b55c2a1 100644
--- a/test/oembed/invalid/oembed-no-width.json
+++ b/test/oembed/oembed-percentage-width.json
@@ -2,5 +2,6 @@
   "version": "1.0",
   "type": "rich",
   "html": "<iframe src='https://example.com/'></iframe>",
+  "width": "100%",
   "height": 300
 }