🐛 Fixed Bluesky URLs creating bookmarks rather than embeds (#20435)

closes #20028 It's fairly common practice for oembed providers to skip some of the "required" fields from the oembed spec such as `height` when it doesn't make sense for the embeddable content, this was the case with Bluesky embeds which return `height: null` - removed validation for `height` being present in the response for it to be recognised as an embed because we don't use it anywhere and the validation is blocking otherwise valid embeds
TryGhost · Jun 20, 2024 · 3bc5eb8 · 3bc5eb8
1 parent 5248fbd
commit 3bc5eb8
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git a/ghost/oembed-service/lib/OEmbedService.js b/ghost/oembed-service/lib/OEmbedService.js
@@ -238,11 +238,11 @@ class OEmbedService {
             scraperResponse = await metascraper({
                 html,
                 url,
-                // In development, allow non-standard tlds
+                // In development, allow non-standard TLDs
                 validateUrl: this.config.get('env') !== 'development'
             });
         } catch (err) {
-            // Log to avoid being blind to errors happenning in metascraper
+            // Log to avoid being blind to errors happening in metascraper
             logging.error(err);
             return this.unknownProvider(url);
         }
@@ -334,7 +334,7 @@ class OEmbedService {
                 if (oembed.type === 'photo' && !oembed.url) {
                     return;
                 }
-                if ((oembed.type === 'video' || oembed.type === 'rich') && (!oembed.html || !oembed.width || !oembed.height)) {
+                if ((oembed.type === 'video' || oembed.type === 'rich') && (!oembed.html || !oembed.width)) {
                     return;
                 }
 

diff --git a/ghost/oembed-service/test/oembed-service.test.js b/ghost/oembed-service/test/oembed-service.test.js
@@ -1,5 +1,6 @@
 const assert = require('assert/strict');
 const nock = require('nock');
+const got = require('got');
 
 const OembedService = require('../');
 
@@ -8,7 +9,12 @@ describe('oembed-service', function () {
     let oembedService;
 
     before(function () {
-        oembedService = new OembedService({});
+        oembedService = new OembedService({
+            config: {get() {
+                return true;
+            }},
+            externalRequest: got
+        });
 
         nock.disableNetConnect();
     });
@@ -96,4 +102,34 @@ describe('oembed-service', function () {
             }
         });
     });
+
+    describe('fetchOembedDataFromUrl', function () {
+        it('allows rich embeds to skip height field', async function () {
+            nock('https://www.example.com')
+                .get('/')
+                .query(true)
+                .reply(200, `<html><head><link type="application/json+oembed" href="https://www.example.com/oembed"></head></html>`);
+
+            nock('https://www.example.com')
+                .get('/oembed')
+                .query(true)
+                .reply(200, {
+                    type: 'rich',
+                    version: '1.0',
+                    title: 'Test Title',
+                    author_name: 'Test Author',
+                    author_url: 'https://example.com/user/testauthor',
+                    html: '<iframe src="https://www.example.com/embed"></iframe>',
+                    width: 640,
+                    height: null
+                });
+
+            const response = await oembedService.fetchOembedDataFromUrl('https://www.example.com');
+
+            assert.equal(response.title, 'Test Title');
+            assert.equal(response.author_name, 'Test Author');
+            assert.equal(response.author_url, 'https://example.com/user/testauthor');
+            assert.equal(response.html, '<iframe src="https://www.example.com/embed"></iframe>');
+        });
+    });
 });