Browse Source

Fix Unfurls for GitHub `og:url` values that are relative.

JC Brand 2 weeks ago
parent
commit
1bfd1a7955

+ 6 - 1
src/headless/shared/parsers.js

@@ -239,9 +239,14 @@ export function getOpenGraphMetadata (stanza) {
                 }
                 return acc;
             }, {
-                'ogp_for_id': applies_to_id,
+                ogp_for_id: applies_to_id,
             });
 
+            const url = data['og:url'];
+            if (url?.startsWith('/') && data['og:site_name']?.toLowerCase() === 'github') {
+                data['og:url'] = `https://github.com${url}`;
+            }
+
             if ("og:description" in data || "og:title" in data || "og:image" in data) {
                 return data;
             }

+ 1 - 1
src/plugins/chatview/tests/message-form.js

@@ -2,7 +2,7 @@
 const { sizzle, u } = converse.env;
 
 describe('A message form', function () {
-    fit(
+    it(
         'can have text pasted into it with automatic space handling',
         mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
             await mock.waitForRoster(_converse, 'current', 1);

+ 36 - 1
src/plugins/muc-views/tests/unfurls.js

@@ -51,6 +51,41 @@ describe("A Groupchat Message", function () {
         expect(unfurl.querySelector('.card-img-top').getAttribute('href')).toBe(unfurl_url);
     }));
 
+    it("will fix GitHub URLs which are relative", mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
+        const nick = 'romeo';
+        const muc_jid = 'lounge@montague.lit';
+        await mock.openAndEnterMUC(_converse, muc_jid, nick);
+        const view = _converse.chatboxviews.get(muc_jid);
+
+        const unfurl_url = 'https://github.com/conversejs/converse.js/commit/b7b14601773e63ede3f93c47550fdc317553a04a';
+
+        const message_stanza = stx`
+            <message xmlns="jabber:client" type="groupchat" from="${muc_jid}/arzu" xml:lang="en" to="${_converse.jid}" id="eda6c790-b4f3-4c07-b5e2-13fff99e6c04">
+                <body>${unfurl_url}</body>
+                <active xmlns="http://jabber.org/protocol/chatstates"/>
+                <origin-id xmlns="urn:xmpp:sid:0" id="eda6c790-b4f3-4c07-b5e2-13fff99e6c04"/>
+                <stanza-id xmlns="urn:xmpp:sid:0" by="${muc_jid}" id="8f7613cc-27d4-40ca-9488-da25c4baf92a"/>
+                <markable xmlns="urn:xmpp:chat-markers:0"/>
+            </message>`;
+        _converse.api.connection.get()._dataRecv(mock.createRequest(message_stanza));
+        const el = await u.waitUntil(() => view.querySelector('.chat-msg__text'));
+        expect(el.textContent).toBe(unfurl_url);
+
+        const metadata_stanza = stx`
+            <message xmlns="jabber:client" from="${muc_jid}" to="${_converse.jid}" type="groupchat">
+                <apply-to xmlns="urn:xmpp:fasten:0" id="eda6c790-b4f3-4c07-b5e2-13fff99e6c04">
+                    <meta xmlns="http://www.w3.org/1999/xhtml" property="og:site_name" content="GitHub" />
+                    <meta xmlns="http://www.w3.org/1999/xhtml" property="og:url" content="https://github.com/conversejs/converse.js/commit/b7b14601773e63ede3f93c47550fdc317553a04a" />
+                    <meta xmlns="http://www.w3.org/1999/xhtml" property="og:title" content="Fix pasting in the message form · conversejs/converse.js@b7b1460" />
+                    <meta xmlns="http://www.w3.org/1999/xhtml" property="og:image" content="https://opengraph.githubassets.com/992a5f409ed015bcb824abb748e04f8feb7bbb92eef7e3da48e08d1e2cf7b053/conversejs/converse.js/commit/b7b14601773e63ede3f93c47550fdc317553a04a" />
+                </apply-to>
+            </message>`;
+        _converse.api.connection.get()._dataRecv(mock.createRequest(metadata_stanza));
+
+        const unfurl = await u.waitUntil(() => view.querySelector('converse-message-unfurl'));
+        expect(unfurl.querySelector('.card-img-top').getAttribute('href')).toBe(unfurl_url);
+    }));
+
     it("will render an unfurl with limited OGP data", mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
         /* Some sites don't include ogp data such as title, description and
          * url. This test is to check that we fall back gracefully */
@@ -218,7 +253,7 @@ describe("A Groupchat Message", function () {
 
     it("will not render an unfurl based on OGP data if render_media is false",
             mock.initConverse(['chatBoxesFetched'],
-            { 'render_media': false },
+            { render_media: false },
             async function (_converse) {
 
         const { api } = _converse;

+ 2 - 1
src/shared/chat/templates/message.js

@@ -39,7 +39,7 @@ export default (el) => {
     // Note: it can happen that the contact has not the vcard attribute but the message has.
     const avatar_model = contact?.vcard ? contact : el.model;
 
-    return html` ${is_first_unread
+    return html`${is_first_unread
             ? html`<div class="message separator">
                   <hr class="separator" />
                   <span class="separator-text">${i18n_new_messages}</span>
@@ -120,6 +120,7 @@ export default (el) => {
                         description="${m['og:description'] || ''}"
                         title="${m['og:title'] || ''}"
                         image="${m['og:image'] || ''}"
+                        site_name="${m['og:site_name'] || ''}"
                         url="${m['og:url'] || ''}"
                     ></converse-message-unfurl>`;
                 })}

+ 54 - 18
src/shared/chat/templates/unfurl.js

@@ -1,37 +1,73 @@
+import { html } from 'lit';
+import log from '@converse/log';
 import { u } from '@converse/headless';
 import { isDomainAllowed } from 'utils/url.js';
-import { html } from 'lit';
 import 'shared/texture/components/image.js';
 
-function isValidImage (image) {
+/**
+ * @param {string} image
+ * @returns {boolean}
+ */
+function isValidImage(image) {
     return image && isDomainAllowed(image, 'allowed_image_domains') && u.isValidURL(image);
 }
 
-const tplUrlWrapper = (o, wrapped_template) =>
-    o.url && u.isValidURL(o.url) && !u.isGIFURL(o.image)
-        ? html`<a href="${o.url}" target="_blank" rel="noopener">${wrapped_template(o)}</a>`
-        : wrapped_template(o);
+/**
+ * @param {import('../unfurl').default} el
+ * @param {(title: string) => import('lit').TemplateResult} wrapped_template
+ */
+function tplUrlWrapper(el, wrapped_template) {
+    const image = el.image || '';
+    const url = el.url || '';
+    const title = el.title || '';
+    return url && u.isValidURL(url) && !u.isGIFURL(image)
+        ? html`<a href="${url}" target="_blank" rel="noopener">${wrapped_template(title)}</a>`
+        : wrapped_template(title ?? '');
+}
 
-const tplImage = (o) =>
-    html`<converse-image class="card-img-top hor_centered" href="${o.url}" src="${o.image}" .onImgLoad=${o.onload}></converse-image>`;
+/**
+ * @param {import('../unfurl').default} el
+ */
+function tplImage(el) {
+    const image = el.image || '';
+    const url = el.url || '';
+    return html`<converse-image
+        class="card-img-top hor_centered"
+        href="${url}"
+        src="${image}"
+        .onImgLoad=${() => el.onImageLoad()}
+    ></converse-image>`;
+}
+/**
+ * @param {import('../unfurl').default} el
+ */
+export default (el) => {
+    const description = el.description || '';
+    const image = el.image || '';
+    const title = el.title || '';
+    const url = el.url || '';
+    const show_image = isValidImage(image);
+    const has_body_info = title || description || url;
 
-export default (o) => {
-    const show_image = isValidImage(o.image);
-    const has_body_info = o.title || o.description || o.url;
-    if (show_image || has_body_info) {
+    if ((show_image || has_body_info)) {
         return html`<div class="card card--unfurl">
-            ${show_image ? tplImage(o) : ''}
+            ${show_image ? tplImage(el) : ''}
             ${has_body_info
                 ? html` <div class="card-body">
-                      ${o.title ? tplUrlWrapper(o, o => html`<h5 class="card-title">${o.title}</h5>`) : ''}
-                      ${o.description
+                      ${title
+                          ? tplUrlWrapper(
+                                el,
+                                /** @param {string} title */ (title) => html`<h5 class="card-title">${title}</h5>`
+                            )
+                          : ''}
+                      ${description
                           ? html`<p class="card-text">
-                                <converse-texture text=${o.description}></converse-texture>
+                                <converse-texture text=${description}></converse-texture>
                             </p>`
                           : ''}
-                      ${o.url
+                      ${url && u.isValidURL(url)
                           ? html`<p class="card-text">
-                                <a href="${o.url}" target="_blank" rel="noopener">${new URL(o.url).hostname}</a>
+                                <a href="${url}" target="_blank" rel="noopener">${new URL(url).hostname}</a>
                             </p>`
                           : ''}
                   </div>`

+ 4 - 14
src/shared/chat/unfurl.js

@@ -12,6 +12,7 @@ export default class MessageUnfurl extends CustomElement {
             jid: { type: String },
             title: { type: String },
             url: { type: String },
+            site_name: { type: String },
         };
     }
 
@@ -22,6 +23,7 @@ export default class MessageUnfurl extends CustomElement {
         this.title = null;
         this.image = null;
         this.description = null;
+        this.site_name = null;
     }
 
     initialize() {
@@ -31,23 +33,11 @@ export default class MessageUnfurl extends CustomElement {
     }
 
     render() {
-        return tplUnfurl(
-            Object.assign(
-                {
-                    'onload': () => this.onImageLoad(),
-                },
-                {
-                    description: this.description || '',
-                    image: this.image || '',
-                    title: this.title || '',
-                    url: this.url || '',
-                }
-            )
-        );
+        return tplUnfurl(this);
     }
 
     onImageLoad() {
-        this.dispatchEvent(new CustomEvent('imageLoaded', { detail: this, 'bubbles': true }));
+        this.dispatchEvent(new CustomEvent('imageLoaded', { detail: this, bubbles: true }));
     }
 }
 

+ 1 - 1
src/shared/components/footer.js

@@ -68,7 +68,7 @@ export class ConverseFooter extends CustomElement {
                                                       href="https://www.keycdn.com?utm_source=conversejs"
                                                       target="_blank"
                                                       rel="noopener"
-                                                      ><img style="height: 3em" src="/logo/keycdn.svg" alt="KeyCDN"
+                                                      ><img style="height: 3em" src="/logo/keycdn.png" alt="KeyCDN"
                                                   /></a>
                                               </div>
                                           </div>

+ 1 - 1
src/types/shared/chat/templates/unfurl.d.ts

@@ -1,3 +1,3 @@
-declare function _default(o: any): "" | import("lit-html").TemplateResult<1>;
+declare function _default(el: import("../unfurl").default): "" | import("lit-html").TemplateResult<1>;
 export default _default;
 //# sourceMappingURL=unfurl.d.ts.map

+ 4 - 0
src/types/shared/chat/unfurl.d.ts

@@ -15,12 +15,16 @@ export default class MessageUnfurl extends CustomElement {
         url: {
             type: StringConstructor;
         };
+        site_name: {
+            type: StringConstructor;
+        };
     };
     jid: any;
     url: any;
     title: any;
     image: any;
     description: any;
+    site_name: any;
     initialize(): void;
     render(): "" | import("lit-html").TemplateResult<1>;
     onImageLoad(): void;