Browse Source

Allow media to be invidually shown/rendered...

even if the global configuration is to disallow it.

* When parsing, include all media URLs, not just the ones from allowed domains.
  That makes it possible to change allowed domains on-the-fly,
  while still allowing media in individual messages to be shown manually
  (via the message actions dropdown).
* Merge `embed_audio`, `embed_video` and `show_images_inline` into `render_media`
* Create new config settings for allowable domains for images, video and audio
* Check the URL domain against a whitelist for the message actions dropdown
JC Brand 3 years ago
parent
commit
efafc2d691

+ 15 - 1
CHANGES.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## 9.0.0 (Unreleased)
+
+- 3 New configuration settings:
+  - [render_media](https://conversejs.org/docs/html/configuration.html#render-media)
+  - [allowed_audio_domains](https://conversejs.org/docs/html/configuration.html#allowed-audio-domains)
+  - [allowed_video_domains](https://conversejs.org/docs/html/configuration.html#allowed-video-domains)
+  - [allowed_image_domains](https://conversejs.org/docs/html/configuration.html#allowed-image-domains)
+
+Three config settings have been obsoleted:
+  - embed_audio
+  - embed_video
+  - show_images_inline
+
+
 ## 8.0.2 (Unreleased)
 
 - #2640: Add `beforeFetchLoginCredentials` hook
@@ -28,7 +42,7 @@
 - #2348: `auto_join_room` not showing the room in `fullscreen` `view_mode`.
 - #2400: Fixes infinite loop bug when appending .png to allowed image urls
 - #2409: Integrate App Badging API for unread messages
-- #2464: New configuration setting [allow-url-history-change](https://conversejs.org/docs/html/configuration.html#allow-url-history-change)
+- #2464: New configuration setting [allow_url_history_change](https://conversejs.org/docs/html/configuration.html#allow-url-history-change)
 - #2497: Bugfix /nick command is not working
 - Add a Description Of A Project (DOAP) file
 - Add ability to deregister nickname when closing a MUC by setting `auto_register_muc_nickname` to `'unregister'`.

+ 71 - 35
docs/source/configuration.rst

@@ -23,6 +23,38 @@ all the available configuration settings.
 Configuration settings
 ======================
 
+.. _`allowed_audio_domains`:
+
+allowed_audio_domains
+---------------------
+
+* Default: ``null``
+
+If falsy, all domains are allowed. Set it to an array to specify a whitelist of allowed domains.
+
+
+.. _`allowed_image_domains`:
+
+allowed_image_domains
+---------------------
+
+* Default: ``null``
+
+If falsy, all domains are allowed. Set it to an array to specify a whitelist of allowed domains.
+
+E.g. ``['imgur.com', 'imgbb.com']``
+
+.. _`allowed_video_domains`:
+
+allowed_video_domains
+---------------------
+
+* Default: ``null``
+
+If falsy, all domains are allowed. Set it to an array to specify a whitelist of allowed domains.
+
+E.g. ``['imgur.com']``
+
 authentication
 --------------
 
@@ -751,14 +783,6 @@ JIDs with other domains are still allowed but need to be provided in full.
 To specify only one domain and disallow other domains, see the `locked_domain`_
 option.
 
-registration_domain
--------------------
-
-* Default: ``''``
-
-Specify a domain name for which the registration form will be fetched automatically,
-without the user having to enter any XMPP server domain name.
-
 default_state
 -------------
 
@@ -793,28 +817,6 @@ domain_placeholder
 The placeholder text shown in the domain input on the registration form.
 
 
-embed_audio
------------
-
-* Default:  ``true``
-
-If set to ``false``, audio files won't be embedded in chats, instead only their links will be shown.
-
-It also accepts an array strings of whitelisted domain names to only render audio files that belong to those domains.
-E.g. ``['conversejs.org']``
-
-
-embed_videos
-------------
-
-* Default:  ``true``
-
-If set to ``false``, videos won't be rendered in chats, instead only their links will be shown.
-
-It also accepts an array strings of whitelisted domain names to only render videos that belong to those domains.
-E.g. ``['imgur.com', 'imgbb.com']``
-
-
 emoji_categories
 ----------------
 
@@ -1449,11 +1451,10 @@ Example:
 muc_show_info_messages
 ----------------------
 
-* Default: List composed of MUC status codes, role changes, join and leave events
-and affiliation changes. The values of converse.MUC_INFO_CODES below are joined to
-build the default list:
+* Default: List composed of MUC status codes, role changes, join and leave events and affiliation changes. The values of converse.MUC_INFO_CODES below are joined to build the default list:
 
 .. code-block:: javascript
+
     converse.MUC_AFFILIATION_CHANGES_LIST = ['owner', 'admin', 'member', 'exowner', 'exadmin', 'exmember', 'exoutcast']
     converse.MUC_ROLE_CHANGES_LIST = ['op', 'deop', 'voice', 'mute'];
     converse.MUC_TRAFFIC_STATES_LIST = ['entered', 'exited'];
@@ -1475,6 +1476,7 @@ It is recommended to use the aforementioned Converse object in the following fas
 to build the list of desired info messages that will be shown:
 
 .. code-block:: javascript
+
     muc_show_info_messages: [
         ...converse.MUC_INFO_CODES.visibility_changes,
         ...converse.MUC_INFO_CODES.self,
@@ -1813,6 +1815,40 @@ For example:
         });
 
 
+registration_domain
+-------------------
+
+* Default: ``''``
+
+Specify a domain name for which the registration form will be fetched automatically,
+without the user having to enter any XMPP server domain name.
+
+render_media
+------------
+
+* Default: ``true``
+
+If ``true``, media files (images, audio and video) will be rendered in the chat.
+Otherwise, only their URLs will be shown.
+
+Note, even if this setting is ``false``, a user can still click on the message
+dropdown and click to show the media for that particular message.
+
+If you want to disable this ability, you can set the allowed domains for that
+media type to an empty array.
+
+See:
+
+* `allowed_audio_domains`_
+* `allowed_video_domains`_
+* `allowed_image_domains`_
+
+.. note::
+
+  This setting, together with the three allowed domain settings above, obsolete
+  the ``show_images_inline``, ``embed_audio`` and ``embed_videos`` settings.
+
+
 .. _`roomconfig_whitelist`:
 
 roomconfig_whitelist
@@ -1973,8 +2009,8 @@ show_images_inline
 
 If set to ``false``, images won't be rendered in chats, instead only their links will be shown.
 
-It also accepts an array strings of whitelisted domain names to only render images that belong to those domains.
-E.g. ``['imgur.com', 'imgbb.com']``
+Users will however still have the ability to render individual images via the message actions dropdown.
+If you want to disallow users from doing so, set the ``allowed_image_domains`` option to an empty array ``[]``.
 
 
 show_retraction_warning

+ 3 - 3
src/headless/plugins/chat/model.js

@@ -9,9 +9,9 @@ import { _converse, api, converse } from "../../core.js";
 import { getOpenPromise } from '@converse/openpromise';
 import { initStorage } from '@converse/headless/utils/storage.js';
 import { debouncedPruneHistory, pruneHistory } from '@converse/headless/shared/chat/utils.js';
-import { getMediaURLs } from '@converse/headless/shared/parsers';
+import { getMediaURLsMetadata } from '@converse/headless/shared/parsers.js';
 import { parseMessage } from './parsers.js';
-import { sendMarker } from '@converse/headless/shared/actions';
+import { sendMarker } from '@converse/headless/shared/actions.js';
 
 const { Strophe, $msg } = converse.env;
 
@@ -870,7 +870,7 @@ const ChatBox = ModelWithContact.extend({
             body,
             is_spoiler,
             origin_id
-        }, getMediaURLs(text));
+        }, getMediaURLsMetadata(text));
     },
 
     /**

+ 2 - 2
src/headless/plugins/chat/parsers.js

@@ -11,7 +11,7 @@ import {
     getCorrectionAttributes,
     getEncryptionAttributes,
     getErrorAttributes,
-    getMediaURLs,
+    getMediaURLsMetadata,
     getOutOfBandAttributes,
     getReceiptId,
     getReferences,
@@ -218,5 +218,5 @@ export async function parseMessage (stanza, _converse) {
     // We call this after the hook, to allow plugins to decrypt encrypted
     // messages, since we need to parse the message text to determine whether
     // there are media urls.
-    return Object.assign(attrs, getMediaURLs(attrs.is_encrypted ? attrs.plaintext : attrs.body));
+    return Object.assign(attrs, getMediaURLsMetadata(attrs.is_encrypted ? attrs.plaintext : attrs.body));
 }

+ 3 - 3
src/headless/plugins/mam/api.js

@@ -34,7 +34,7 @@ export default {
           */
 
          /**
-          * The options that can be passed in to the { @link _converse.api.archive.query } method
+          * The options that can be passed in to the {@link _converse.api.archive.query } method
           * @typedef { module:converse-mam~MAMFilterParameters } ArchiveQueryOptions
           * @property { Boolean } [groupchat=false] - Whether the MAM archive is for a groupchat.
           */
@@ -49,7 +49,7 @@ export default {
           * @param { module:converse-mam~ArchiveQueryOptions } options - An object containing query parameters
           * @throws {Error} An error is thrown if the XMPP server responds with an error.
           * @returns { Promise<module:converse-mam~MAMQueryResult> } A promise which resolves
-          *     to a { @link module:converse-mam~MAMQueryResult } object.
+          *     to a {@link module:converse-mam~MAMQueryResult } object.
           *
           * @example
           * // Requesting all archived messages
@@ -292,7 +292,7 @@ export default {
             /**
              * @typedef { Object } MAMQueryResult
              * @property { Array } messages
-             * @property { RSM } [rsm] - An instance of { @link RSM }.
+             * @property { RSM } [rsm] - An instance of {@link RSM}.
              *  You can call `next()` or `previous()` on this instance,
              *  to get the RSM query parameters for the next or previous
              *  page in the result set.

+ 6 - 6
src/headless/plugins/muc/muc.js

@@ -13,7 +13,7 @@ import { _converse, api, converse } from '../../core.js';
 import { computeAffiliationsDelta, setAffiliations, getAffiliationList }  from './affiliations/utils.js';
 import { getOpenPromise } from '@converse/openpromise';
 import { initStorage } from '@converse/headless/utils/storage.js';
-import { isArchived, getMediaURLs } from '@converse/headless/shared/parsers';
+import { isArchived, getMediaURLsMetadata } from '@converse/headless/shared/parsers';
 import { parseMUCMessage, parseMUCPresence } from './parsers.js';
 import { sendMarker } from '@converse/headless/shared/actions';
 
@@ -554,8 +554,8 @@ const ChatRoomMixin = {
         }
         /**
          * @typedef { Object } MUCMessageData
-         * An object containing the parsed { @link MUCMessageAttributes } and
-         * current { @link ChatRoom }.
+         * An object containing the parsed {@link MUCMessageAttributes} and
+         * current {@link ChatRoom}.
          * @property { MUCMessageAttributes } attrs
          * @property { ChatRoom } chatbox
          */
@@ -996,7 +996,7 @@ const ChatRoomMixin = {
             'nick': this.get('nick'),
             'sender': 'me',
             'type': 'groupchat'
-        }, getMediaURLs(text));
+        }, getMediaURLsMetadata(text));
     },
 
     /**
@@ -2107,7 +2107,7 @@ const ChatRoomMixin = {
     },
 
     /**
-     * Given { @link MessageAttributes } look for XEP-0316 Room Notifications and create info
+     * Given {@link MessageAttributes} look for XEP-0316 Room Notifications and create info
      * messages for them.
      * @param { XMLElement } stanza
      */
@@ -2129,7 +2129,7 @@ const ChatRoomMixin = {
      * passed in attributes map.
      * @method _converse.ChatRoom#getDuplicateMessage
      * @param { object } attrs - Attributes representing a received
-     *  message, as returned by { @link parseMUCMessage }
+     *  message, as returned by {@link parseMUCMessage}
      * @returns {Promise<_converse.Message>}
      */
     getDuplicateMessage (attrs) {

+ 2 - 2
src/headless/plugins/muc/parsers.js

@@ -6,7 +6,7 @@ import {
     getCorrectionAttributes,
     getEncryptionAttributes,
     getErrorAttributes,
-    getMediaURLs,
+    getMediaURLsMetadata,
     getOpenGraphMetadata,
     getOutOfBandAttributes,
     getReceiptId,
@@ -251,7 +251,7 @@ export async function parseMUCMessage (stanza, chatbox, _converse) {
     // We call this after the hook, to allow plugins to decrypt encrypted
     // messages, since we need to parse the message text to determine whether
     // there are media urls.
-    return Object.assign(attrs, getMediaURLs(attrs.is_encrypted ? attrs.plaintext : attrs.body));
+    return Object.assign(attrs, getMediaURLsMetadata(attrs.is_encrypted ? attrs.plaintext : attrs.body));
 }
 
 /**

+ 32 - 0
src/headless/shared/chat/utils.js

@@ -26,4 +26,36 @@ export function pruneHistory (model) {
     }
 }
 
+/**
+ * Given an array of {@link MediaURLMetadata} objects and text, return an
+ * array of {@link MediaURL} objects.
+ * @param { Array<MediaURLMetadata> } arr
+ * @param { String } text
+ * @returns{ Array<MediaURL> }
+ */
+export function getMediaURLs (arr, text, offset=0) {
+    /**
+     * @typedef { Object } MediaURLData
+     * An object representing a URL found in a chat message
+     * @property { Boolean } is_audio
+     * @property { Boolean } is_image
+     * @property { Boolean } is_video
+     * @property { String } end
+     * @property { String } start
+     * @property { String } url
+     */
+    return arr.map(o => {
+        const start = o.start - offset;
+        const end = o.end - offset;
+        if (start < 0 || start >= text.length) {
+            return null;
+        }
+        return Object.assign({}, o, {
+            start,
+            end,
+            'url': text.substring(o.start-offset, o.end-offset),
+        });
+    }).filter(o => o);
+}
+
 export const debouncedPruneHistory = debounce(pruneHistory, 250);

+ 1 - 1
src/headless/shared/constants.js

@@ -37,4 +37,4 @@ export const CORE_PLUGINS = [
     'converse-vcard'
 ];
 
-export const URL_PARSE_OPTIONS = { 'start': /\b(?:([a-z][a-z0-9.+-]*:\/\/)|xmpp:|mailto:|www\.)/gi };
+export const URL_PARSE_OPTIONS = { 'start': /(\b|_)(?:([a-z][a-z0-9.+-]*:\/\/)|xmpp:|mailto:|www\.)/gi };

+ 31 - 9
src/headless/shared/parsers.js

@@ -8,11 +8,9 @@ import { _converse, api } from '@converse/headless/core';
 import { decodeHTMLEntities } from '@converse/headless/utils/core.js';
 import { rejectMessage } from '@converse/headless/shared/actions';
 import {
-    isAudioDomainAllowed,
     isAudioURL,
-    isImageDomainAllowed,
+    isEncryptedFileURL,
     isImageURL,
-    isVideoDomainAllowed,
     isVideoURL
 } from '@converse/headless/utils/url.js';
 
@@ -178,7 +176,7 @@ export function getOpenGraphMetadata (stanza) {
 }
 
 
-export function getMediaURLs (text) {
+export function getMediaURLsMetadata (text) {
     const objs = [];
     if (!text) {
         return {};
@@ -187,6 +185,14 @@ export function getMediaURLs (text) {
         URI.withinString(
             text,
             (url, start, end) => {
+                if (url.startsWith('_')) {
+                    url = url.slice(1);
+                    start += 1;
+                }
+                if (url.endsWith('_')) {
+                    url = url.slice(0, url.length-1);
+                    end -= 1;
+                }
                 objs.push({ url, start, end });
                 return url;
             },
@@ -195,11 +201,27 @@ export function getMediaURLs (text) {
     } catch (error) {
         log.debug(error);
     }
-    const media_urls = objs.filter(o => {
-        return (isImageURL(o.url) && isImageDomainAllowed(o.url)) ||
-           (isVideoURL(o.url) && isVideoDomainAllowed(o.url)) ||
-            (isAudioURL(o.url) && isAudioDomainAllowed(o.url));
-    }).map(o => ({ 'start': o.start, 'end': o.end }));
+
+    /**
+     * @typedef { Object } MediaURLMetadata
+     * An object representing the metadata of a URL found in a chat message
+     * The actual URL is not saved, it can be extracted via the `start` and `end` indexes.
+     * @property { Boolean } is_audio
+     * @property { Boolean } is_image
+     * @property { Boolean } is_video
+     * @property { String } end
+     * @property { String } start
+     */
+    const media_urls = objs
+        .map(o => ({
+            'end': o.end,
+            'is_audio': isAudioURL(o.url),
+            'is_image': isImageURL(o.url),
+            'is_video': isVideoURL(o.url),
+            'is_encrypted': isEncryptedFileURL(o.url),
+            'start': o.start
+
+        }));
     return media_urls.length ? { media_urls } : {};
 }
 

+ 1 - 1
src/headless/shared/rsm.js

@@ -82,7 +82,7 @@ export class RSM {
 
     /**
      * Returns a `<set>` XML element that confirms to XEP-0059 Result Set Management.
-     * The element is constructed based on the { @link module:converse-rsm~RSMQueryParameters }
+     * The element is constructed based on the {@link module:converse-rsm~RSMQueryParameters}
      * that are set on this RSM instance.
      * @returns { XMLElement }
      */

+ 1 - 1
src/headless/utils/storage.js

@@ -25,7 +25,7 @@ export function createStore (id, store) {
 
 export function initStorage (model, id, type) {
     const store = type || getDefaultStore();
-    model.browserStorage = _converse.createStore(id, store);
+    model.browserStorage = createStore(id, store);
     if (storeUsesIndexedDB(store)) {
         const flush = () => model.browserStorage.flush();
         window.addEventListener(_converse.unloadevent, flush);

+ 16 - 30
src/headless/utils/url.js

@@ -28,7 +28,7 @@ function checkFileTypes (types, url) {
     return !!types.filter(ext => filename.endsWith(ext)).length;
 }
 
-function isDomainAllowed (whitelist, url) {
+function isDomainWhitelisted (whitelist, url) {
     const uri = getURI(url);
     const subdomain = uri.subdomain();
     const domain = uri.domain();
@@ -43,43 +43,29 @@ export function filterQueryParamsFromURL (url) {
     return parsed_uri.removeQuery(paramsArray).toString();
 }
 
-export function isAudioDomainAllowed (url) {
-    const embed_audio = api.settings.get('embed_audio');
-    if (!Array.isArray(embed_audio)) {
-        return embed_audio;
+export function isDomainAllowed (url, setting) {
+    const allowed_domains = api.settings.get(setting);
+    if (!Array.isArray(allowed_domains)) {
+        return true;
     }
     try {
-        return isDomainAllowed(embed_audio, url);
+        return isDomainWhitelisted(allowed_domains, url);
     } catch (error) {
         log.debug(error);
         return false;
     }
 }
 
-export function isVideoDomainAllowed (url) {
-    const embed_videos = api.settings.get('embed_videos');
-    if (!Array.isArray(embed_videos)) {
-        return embed_videos;
-    }
-    try {
-        return isDomainAllowed(embed_videos, url);
-    } catch (error) {
-        log.debug(error);
-        return false;
-    }
-}
-
-export function isImageDomainAllowed (url) {
-    const show_images_inline = api.settings.get('show_images_inline');
-    if (!Array.isArray(show_images_inline)) {
-        return show_images_inline;
-    }
-    try {
-        return isDomainAllowed(show_images_inline, url);
-    } catch (error) {
-        log.debug(error);
-        return false;
-    }
+/**
+ * Accepts a {@link MediaURL} object and then checks whether its domain is
+ * allowed for rendering in the chat.
+ * @param { MediaURL } o
+ * @returns { Bool }
+ */
+export function isMediaURLDomainAllowed (o) {
+    return o.is_audio && isDomainAllowed(o.url, 'allowed_audio_domains') ||
+        o.is_video && isDomainAllowed(o.url, 'allowed_video_domains') ||
+        o.is_image && isDomainAllowed(o.url, 'allowed_image_domains');
 }
 
 export function isURLWithImageExtension (url) {

+ 4 - 3
src/plugins/chatview/index.js

@@ -34,15 +34,16 @@ converse.plugins.add('converse-chatview', {
          * loaded by converse.js's plugin machinery.
          */
         api.settings.extend({
+            'allowed_audio_domains': null,
+            'allowed_image_domains': null,
+            'allowed_video_domains': null,
             'auto_focus': true,
             'debounced_content_rendering': true,
-            'embed_videos': true,
-            'embed_audio': true,
             'filter_url_query_params': null,
             'image_urls_regex': null,
             'message_limit': 0,
             'muc_hats': ['xep317'],
-            'show_images_inline': true,
+            'render_media': true,
             'show_message_avatar': true,
             'show_retraction_warning': true,
             'show_send_button': true,

+ 22 - 10
src/plugins/chatview/tests/message-images.js

@@ -51,8 +51,8 @@ describe("A Chat Message", function () {
         await u.waitUntil(() => Array.from(view.querySelectorAll('.chat-content .chat-image')).pop().src.endsWith('png'), 1000);
     }));
 
-    it("will not render images if show_images_inline is false",
-            mock.initConverse(['chatBoxesFetched'], {'show_images_inline': false}, async function (_converse) {
+    it("will not render images if render_media is false",
+            mock.initConverse(['chatBoxesFetched'], {'render_media': false}, async function (_converse) {
         await mock.waitForRoster(_converse, 'current');
         const base_url = 'https://conversejs.org';
         const message = base_url+"/logo/conversejs-filled.svg";
@@ -68,7 +68,7 @@ describe("A Chat Message", function () {
 
     it("will render images from approved URLs only",
         mock.initConverse(
-            ['chatBoxesFetched'], {'show_images_inline': ['conversejs.org']},
+            ['chatBoxesFetched'], {'render_media': ['conversejs.org']},
             async function (_converse) {
 
         await mock.waitForRoster(_converse, 'current');
@@ -111,7 +111,7 @@ describe("A Chat Message", function () {
     it("will fall back to rendering URLs that match image_urls_regex as URLs",
         mock.initConverse(
             ['rosterGroupsFetched', 'chatBoxesFetched'], {
-                'show_images_inline': ['twimg.com'],
+                'render_media': ['twimg.com'],
                 'image_urls_regex': /^https?:\/\/(www.)?(pbs\.twimg\.com\/)/i
             },
             async function (_converse) {
@@ -130,9 +130,9 @@ describe("A Chat Message", function () {
             `<a target="_blank" rel="noopener" href="https://pbs.twimg.com/media/string?format=jpg&amp;name=small">https://pbs.twimg.com/media/string?format=jpg&amp;name=small</a>`, 1000);
     }));
 
-    it("will respect a changed setting when re-rendered",
+    it("will respect a changed allowed_image_domains setting when re-rendered",
         mock.initConverse(
-            ['chatBoxesFetched'], {'show_images_inline': true},
+            ['chatBoxesFetched'], {'render_media': true},
             async function (_converse) {
 
         const { api } = _converse;
@@ -143,14 +143,26 @@ describe("A Chat Message", function () {
         const view = _converse.chatboxviews.get(contact_jid);
         await mock.sendMessage(view, message);
         await u.waitUntil(() => view.querySelectorAll('converse-chat-message-body .chat-image').length === 1);
-        api.settings.set('show_images_inline', false);
-        view.querySelector('converse-chat-message').requestUpdate();
+        expect(view.querySelector('.chat-msg__action-hide-previews')).not.toBe(null);
+
+        api.settings.set('allowed_image_domains', []);
+
+        // FIXME: remove once we can update based on settings change event
+        view.querySelector('converse-chat-message-body').requestUpdate();
+        view.querySelector('converse-message-actions').requestUpdate();
         await u.waitUntil(() => view.querySelector('converse-chat-message-body .chat-image') === null);
-        expect(true).toBe(true);
+        expect(view.querySelector('.chat-msg__action-hide-previews')).toBe(null);
+
+        // FIXME: remove once we can update based on settings change event
+        api.settings.set('allowed_image_domains', null);
+        view.querySelector('converse-chat-message-body').requestUpdate();
+        view.querySelector('converse-message-actions').requestUpdate();
+        await u.waitUntil(() => view.querySelector('converse-chat-message-body .chat-image'));
+        expect(view.querySelector('.chat-msg__action-hide-previews')).not.toBe(null);
     }));
 
     it("will allow the user to toggle visibility of rendered images",
-            mock.initConverse(['chatBoxesFetched'], {'show_images_inline': true}, async function (_converse) {
+            mock.initConverse(['chatBoxesFetched'], {'render_media': true}, async function (_converse) {
 
         await mock.waitForRoster(_converse, 'current');
         // let message = "https://i.imgur.com/Py9ifJE.mp4";

+ 4 - 4
src/plugins/chatview/tests/message-videos.js

@@ -29,8 +29,8 @@ describe("A chat message containing video URLs", function () {
             `<a target="_blank" rel="noopener" href="${Strophe.xmlescape(message)}">${Strophe.xmlescape(message)}</a>`);
     }));
 
-    it("will not render videos if embed_videos is false",
-            mock.initConverse(['chatBoxesFetched'], {'embed_videos': false}, async function (_converse) {
+    it("will not render videos if render_media is false",
+            mock.initConverse(['chatBoxesFetched'], {'render_media': false}, async function (_converse) {
         await mock.waitForRoster(_converse, 'current');
         // let message = "https://i.imgur.com/Py9ifJE.mp4";
         const base_url = 'https://conversejs.org';
@@ -47,7 +47,7 @@ describe("A chat message containing video URLs", function () {
 
     it("will render videos from approved URLs only",
         mock.initConverse(
-            ['chatBoxesFetched'], {'embed_videos': ['conversejs.org']},
+            ['chatBoxesFetched'], {'allowed_video_domains': ['conversejs.org']},
             async function (_converse) {
 
         await mock.waitForRoster(_converse, 'current');
@@ -70,7 +70,7 @@ describe("A chat message containing video URLs", function () {
     }));
 
     it("will allow the user to toggle visibility of rendered videos",
-            mock.initConverse(['chatBoxesFetched'], {'embed_videos': true}, async function (_converse) {
+            mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
 
         await mock.waitForRoster(_converse, 'current');
         // let message = "https://i.imgur.com/Py9ifJE.mp4";

+ 1 - 1
src/plugins/muc-views/muc.js

@@ -25,7 +25,7 @@ export default class MUCView extends BaseChatView {
         this.onConnectionStatusChanged();
         this.model.maybeShow();
         /**
-         * Triggered once a { @link _converse.ChatRoomView } has been opened
+         * Triggered once a {@link _converse.ChatRoomView} has been opened
          * @event _converse#chatRoomViewInitialized
          * @type { _converse.ChatRoomView }
          * @example _converse.api.listen.on('chatRoomViewInitialized', view => { ... });

+ 3 - 3
src/plugins/muc-views/tests/unfurls.js

@@ -254,9 +254,9 @@ describe("A Groupchat Message", function () {
         expect(unfurls.length).toBe(1);
     }));
 
-    it("will not render an unfurl image if the domain is not in show_images_inline",
+    it("will not render an unfurl image if the domain is not in allowed_image_domains",
             mock.initConverse(['chatBoxesFetched'],
-            {'show_images_inline': []},
+            {'allowed_image_domains': []},
             async function (_converse) {
 
         const nick = 'romeo';
@@ -294,7 +294,7 @@ describe("A Groupchat Message", function () {
 
     it("lets the user hide an unfurl",
             mock.initConverse(['chatBoxesFetched'],
-            {'show_images_inline': []},
+            {'render_media': []},
             async function (_converse) {
 
         const nick = 'romeo';

+ 20 - 6
src/shared/chat/message-actions.js

@@ -1,8 +1,10 @@
 import log from '@converse/headless/log';
 import { CustomElement } from 'shared/components/element.js';
 import { __ } from 'i18n';
-import { _converse, api, converse } from '@converse/headless/core';
+import { _converse, api, converse } from '@converse/headless/core.js';
+import { getMediaURLs } from '@converse/headless/shared/chat/utils.js';
 import { html } from 'lit';
+import { isMediaURLDomainAllowed } from '@converse/headless/utils/url.js';
 import { until } from 'lit/directives/until.js';
 
 const { Strophe, u } = converse.env;
@@ -218,17 +220,29 @@ class MessageActions extends CustomElement {
         }
         const ogp_metadata = this.model.get('ogp_metadata') || [];
         const unfurls_to_show = api.settings.get('muc_show_ogp_unfurls') && ogp_metadata.length;
-        const media_to_show = this.model.get('media_urls')?.length;
-
+        const media_urls = getMediaURLs(this.model.get('media_urls') || [], this.model.get('body'));
+        const media_to_show = media_urls.reduce((result, o) => result || isMediaURLDomainAllowed(o), false);
         if (unfurls_to_show || media_to_show) {
             let title;
             const hidden_preview = this.hide_url_previews;
             if (ogp_metadata.length > 1) {
-                title = hidden_preview ? __('Show URL previews') : __('Hide URL previews');
+                if (typeof hidden_preview === 'boolean') {
+                    title = hidden_preview ? __('Show URL previews') : __('Hide URL previews');
+                } else {
+                    title = api.settings.get('render_media') ? __('Hide URL previews') : __('Show URL previews');
+                }
             } else if (ogp_metadata.length === 1) {
-                title = hidden_preview ? __('Show URL preview') : __('Hide URL preview');
+                if (typeof hidden_preview === 'boolean') {
+                    title = hidden_preview ? __('Show URL preview') : __('Hide URL preview');
+                } else {
+                    title = api.settings.get('render_media') ? __('Hide URL previews') : __('Show URL previews');
+                }
             } else  {
-                title = hidden_preview ? __('Show media') : __('Hide media');
+                if (typeof hidden_preview === 'boolean') {
+                    title = hidden_preview ? __('Show media') : __('Hide media');
+                } else {
+                    title = api.settings.get('render_media') ? __('Hide media') : __('Show media');
+                }
             }
             buttons.push({
                 'i18n_text': title,

+ 17 - 11
src/shared/chat/message-body.js

@@ -11,12 +11,12 @@ export default class MessageBody extends CustomElement {
 
     static get properties () {
         return {
-            embed_audio: { type: Boolean },
-            embed_videos: { type: Boolean },
-            hide_url_previews: { type: Boolean },
+            // We make this a string instead of a boolean, since we want to
+            // distinguish between true, false and undefined states
+            hide_url_previews: { type: String },
             is_me_message: { type: Boolean },
             model: { type: Object },
-            show_images: { type: Boolean },
+            render_media: { type: Boolean },
             text: { type: String },
         }
     }
@@ -33,19 +33,25 @@ export default class MessageBody extends CustomElement {
     render () {
         const callback = () => this.model.collection?.trigger('rendered', this.model);
         const offset = 0;
-        const mentions = this.model.get('references');
-
         const options = {
-            'embed_audio': !this.hide_url_previews && this.embed_audio,
-            'embed_videos': !this.hide_url_previews && this.embed_videos,
+            'media_urls': this.model.get('media_urls'),
+            'mentions': this.model.get('references'),
             'nick': this.model.collection.chatbox.get('nick'),
             'onImgClick': (ev) => this.onImgClick(ev),
             'onImgLoad': () => this.onImgLoad(),
             'render_styling': !this.model.get('is_unstyled') && api.settings.get('allow_message_styling'),
-            'show_images': !this.hide_url_previews && this.show_images,
-            'show_me_message': true
+            'show_me_message': true,
+        }
+        if (this.hide_url_previews === "false") {
+            options.embed_audio = true;
+            options.embed_videos = true;
+            options.show_images = true;
+        } else if (this.hide_url_previews === "true") {
+            options.embed_audio = false;
+            options.embed_videos = false;
+            options.show_images = false;
         }
-        return renderRichText(this.text, offset, mentions, options, callback);
+        return renderRichText(this.text, offset, options, callback);
     }
 }
 

+ 2 - 4
src/shared/chat/templates/message-text.js

@@ -33,11 +33,9 @@ export default (el) => {
             <converse-chat-message-body
                 class="chat-msg__text ${el.model.get('is_only_emojis') ? 'chat-msg__text--larger' : ''} ${spoiler_classes}"
                 .model="${el.model}"
-                ?hide_url_previews=${el.model.get('hide_url_previews')}
+                hide_url_previews=${el.model.get('hide_url_previews')}
                 ?is_me_message=${el.model.isMeCommand()}
-                ?show_images=${api.settings.get('show_images_inline')}
-                ?embed_videos=${api.settings.get('embed_videos')}
-                ?embed_audio=${api.settings.get('embed_audio')}
+                ?render_media=${api.settings.get('render_media')}
                 text="${text}"></converse-chat-message-body>
             ${ (el.model.get('received') && !el.model.isMeCommand() && !is_groupchat_message) ? html`<span class="fa fa-check chat-msg__receipt"></span>` : '' }
             ${ (el.model.get('edited')) ? tpl_edited_icon(el) : '' }

+ 2 - 2
src/shared/chat/templates/unfurl.js

@@ -1,4 +1,4 @@
-import { getURI, isAudioURL, isGIFURL, isVideoURL, isImageDomainAllowed } from '@converse/headless/utils/url.js';
+import { getURI, isAudioURL, isGIFURL, isVideoURL, isDomainAllowed } from '@converse/headless/utils/url.js';
 import { html } from 'lit';
 
 
@@ -8,7 +8,7 @@ function isValidURL (url) {
 }
 
 function isValidImage (image) {
-    return image && isImageDomainAllowed(image) && isValidURL(image);
+    return image && isDomainAllowed(image, 'allowed_image_domains') && isValidURL(image);
 }
 
 function shouldHideMediaURL (o) {

+ 3 - 2
src/shared/components/rich-text.js

@@ -65,15 +65,16 @@ export default class RichText extends CustomElement {
         const options = {
             embed_audio: this.embed_audio,
             embed_videos: this.embed_videos,
+            hide_media_urls: this.hide_media_urls,
+            mentions: this.mentions,
             nick: this.nick,
             onImgClick: this.onImgClick,
             onImgLoad: this.onImgLoad,
             render_styling: this.render_styling,
             show_images: this.show_images,
             show_me_message: this.show_me_message,
-            hide_media_urls: this.hide_media_urls,
         }
-        return renderRichText(this.text, this.offset, this.mentions, options);
+        return renderRichText(this.text, this.offset, options);
     }
 }
 

+ 10 - 6
src/shared/directives/rich-text.js

@@ -1,3 +1,4 @@
+import log from '@converse/headless/log.js';
 import { Directive, directive } from 'lit/directive.js';
 import { RichText } from 'shared/rich-text.js';
 import { html } from "lit";
@@ -6,16 +7,19 @@ import { until } from 'lit/directives/until.js';
 
 class RichTextRenderer {
 
-    constructor (text, offset, mentions=[], options={}) {
-        this.mentions = mentions;
+    constructor (text, offset, options={}) {
         this.offset = offset;
         this.options = options;
         this.text = text;
     }
 
     async transform () {
-        const text = new RichText(this.text, this.offset, this.mentions, this.options);
-        await text.addTemplates();
+        const text = new RichText(this.text, this.offset, this.options);
+        try {
+            await text.addTemplates();
+        } catch (e) {
+            log.error(e);
+        }
         return text.payload;
     }
 
@@ -26,8 +30,8 @@ class RichTextRenderer {
 
 
 class RichTextDirective extends Directive {
-    render (text, offset, mentions, options, callback) { // eslint-disable-line class-methods-use-this
-        const renderer = new RichTextRenderer(text, offset, mentions, options);
+    render (text, offset, options, callback) { // eslint-disable-line class-methods-use-this
+        const renderer = new RichTextRenderer(text, offset, options);
         const result = renderer.render();
         callback?.();
         return result;

+ 7 - 3
src/shared/directives/styling.js

@@ -1,19 +1,23 @@
+import log from '@converse/headless/log.js';
 import { Directive, directive } from 'lit/directive.js';
 import { RichText } from 'shared/rich-text.js';
 import { html } from 'lit';
 import { until } from 'lit/directives/until.js';
 
 async function transform (t) {
-    await t.addTemplates();
+    try {
+        await t.addTemplates();
+    } catch (e) {
+        log.error(e);
+    }
     return t.payload;
 }
 
 class StylingDirective extends Directive {
-    render (txt, offset, mentions, options) { // eslint-disable-line class-methods-use-this
+    render (txt, offset, options) { // eslint-disable-line class-methods-use-this
         const t = new RichText(
             txt,
             offset,
-            mentions,
             Object.assign(options, { 'show_images': false, 'embed_videos': false, 'embed_audio': false })
         );
         return html`${until(transform(t), html`${t}`)}`;

+ 54 - 39
src/shared/rich-text.js

@@ -1,12 +1,12 @@
-import log from '@converse/headless/log';
 import tpl_audio from 'templates/audio.js';
 import tpl_gif from 'templates/gif.js';
 import tpl_image from 'templates/image.js';
 import tpl_video from 'templates/video.js';
-import { URL_PARSE_OPTIONS } from '@converse/headless/shared/constants.js';
-import { _converse, api, converse } from '@converse/headless/core';
+import { _converse, api } from '@converse/headless/core';
 import { containsDirectives, getDirectiveAndLength, getDirectiveTemplate, isQuoteDirective } from './styling.js';
 import { getHyperlinkTemplate } from 'utils/html.js';
+import { getMediaURLs } from '@converse/headless/shared/chat/utils.js';
+import { getMediaURLsMetadata } from '@converse/headless/shared/parsers.js';
 import {
     convertASCII2Emoji,
     getCodePointReferences,
@@ -15,20 +15,15 @@ import {
 } from '@converse/headless/plugins/emoji/index.js';
 import {
     filterQueryParamsFromURL,
-    isAudioDomainAllowed,
     isAudioURL,
-    isEncryptedFileURL,
+    isDomainAllowed,
     isGIFURL,
-    isImageDomainAllowed,
     isImageURL,
-    isVideoDomainAllowed,
     isVideoURL
 } from '@converse/headless/utils/url.js';
 
 import { html } from 'lit';
 
-const { URI } = converse.env;
-
 const isString = s => typeof s === 'string';
 
 // We don't render more than two line-breaks, replace extra line-breaks with
@@ -63,21 +58,35 @@ export class RichText extends String {
      *  from the start of the original message text. This is necessary because
      *  RichText instances can be nested when templates call directives
      *  which create new RichText instances (as happens with XEP-393 styling directives).
-     * @param { Array } mentions - An array of mention references
      * @param { Object } options
      * @param { String } options.nick - The current user's nickname (only relevant if the message is in a XEP-0045 MUC)
      * @param { Boolean } options.render_styling - Whether XEP-0393 message styling should be applied to the message
-     * @param { Boolean } options.show_images - Whether image URLs should be rendered as <img> tags.
-     * @param { Boolean } options.embed_videos - Whether video URLs should be rendered as <video> tags.
+     * @param { Boolean } [options.embed_audio] - Whether audio URLs should be rendered as <audio> elements.
+     *  If set to `true`, then audio files will always be rendered with an
+     *  audio player. If set to `false`, they won't, and if not defined, then the `embed_audio` setting
+     *  is used to determine whether they should be rendered as playable audio or as hyperlinks.
+     * @param { Boolean } [options.embed_videos] - Whether video URLs should be rendered as <video> elements.
+     *  If set to `true`, then videos will always be rendered with a video
+     *  player. If set to `false`, they won't, and if not defined, then the `embed_videos` setting
+     *  is used to determine whether they should be rendered as videos or as hyperlinks.
+     * @param { Array } [options.mentions] - An array of mention references
+     * @param { Array } [options.media_urls] - An array of {@link MediaURLMetadata} objects,
+     *  used to render media such as images, videos and audio. It might not be
+     *  possible to have the media metadata available, so if this value is
+     *  `undefined` then the passed-in `text` will be parsed for URLs. If you
+     *  don't want this parsing to happen, pass in an empty array for this
+     *  option.
+     * @param { Boolean } [options.show_images] - Whether image URLs should be rendered as <img> elements.
      * @param { Boolean } options.show_me_message - Whether /me messages should be rendered differently
      * @param { Function } options.onImgClick - Callback for when an inline rendered image has been clicked
      * @param { Function } options.onImgLoad - Callback for when an inline rendered image has been loaded
      */
-    constructor (text, offset = 0, mentions = [], options = {}) {
+    constructor (text, offset = 0, options = {}) {
         super(text);
         this.embed_audio = options?.embed_audio;
         this.embed_videos = options?.embed_videos;
-        this.mentions = mentions;
+        this.mentions = options?.mentions || [];
+        this.media_urls = options?.media_urls;
         this.nick = options?.nick;
         this.offset = offset;
         this.onImgClick = options?.onImgClick;
@@ -90,36 +99,42 @@ export class RichText extends String {
         this.hide_media_urls = options?.hide_media_urls;
     }
 
+    shouldRenderMedia (url_text, type) {
+        let override;
+        if (type === 'image') {
+            override = this.show_images;
+        } else if (type === 'audio') {
+            override = this.embed_audio;
+        } else if (type === 'video') {
+            override = this.embed_videos;
+        }
+        if (typeof override === 'boolean') {
+            return override;
+        }
+        const may_render = api.settings.get('render_media');
+        return may_render && isDomainAllowed(url_text, `allowed_${type}_domains`);
+    }
+
     /**
      * Look for `http` URIs and return templates that render them as URL links
      * @param { String } text
-     * @param { Integer } offset - The index of the passed in text relative to
-     *  the start of the message body text.
+     * @param { Integer } local_offset - The index of the passed in text relative to
+     *  the start of this RichText instance (which is not necessarily the same as the
+     *  offset from the start of the original message stanza's body text).
      */
-    addHyperlinks (text, offset) {
-        const objs = [];
-        try {
-            URI.withinString(
-                text,
-                (url, start, end) => {
-                    objs.push({ url, start, end });
-                    return url;
-                },
-                URL_PARSE_OPTIONS
-            );
-        } catch (error) {
-            log.debug(error);
-            return;
-        }
+    addHyperlinks (text, local_offset) {
+        const full_offset = local_offset + this.offset;
+        const urls_meta = this.media_urls || getMediaURLsMetadata(text).media_urls || [];
+        const media_urls = getMediaURLs(urls_meta, text, full_offset);
 
-        objs.filter(o => !isEncryptedFileURL(text.slice(o.start, o.end))).forEach(url_obj => {
+        media_urls.filter(o => !o.is_encrypted).forEach(url_obj => {
             const url_text = url_obj.url;
             const filtered_url = filterQueryParamsFromURL(url_text);
-            let template;
 
-            if (this.show_images && isGIFURL(url_text) && isImageDomainAllowed(url_text)) {
+            let template;
+            if (isGIFURL(url_text) && this.shouldRenderMedia(url_text, 'image')) {
                 template = tpl_gif(filtered_url, this.hide_media_urls);
-            } else if (this.show_images && isImageURL(url_text) && isImageDomainAllowed(url_text)) {
+            } else if (isImageURL(url_text) && this.shouldRenderMedia(url_text, 'image')) {
                 template = tpl_image({
                     'url': filtered_url,
                     // XXX: bit of an abuse of `hide_media_urls`, might want a dedicated option here
@@ -127,14 +142,14 @@ export class RichText extends String {
                     'onClick': this.onImgClick,
                     'onLoad': this.onImgLoad
                 });
-            } else if (this.embed_videos && isVideoURL(url_text) && isVideoDomainAllowed(url_text)) {
+            } else if (isVideoURL(url_text) && this.shouldRenderMedia(url_text, 'video')) {
                 template = tpl_video(filtered_url, this.hide_media_urls);
-            } else if (this.embed_audio && isAudioURL(url_text) && isAudioDomainAllowed(url_text)) {
+            } else if (isAudioURL(url_text) && this.shouldRenderMedia(url_text, 'audio')) {
                 template = tpl_audio(filtered_url, this.hide_media_urls);
             } else {
                 template = getHyperlinkTemplate(filtered_url);
             }
-            this.addTemplateResult(url_obj.start + offset, url_obj.end + offset, template);
+            this.addTemplateResult(url_obj.start + local_offset, url_obj.end + local_offset, template);
         });
     }
 
@@ -226,7 +241,7 @@ export class RichText extends String {
                     const text = this.slice(slice_begin, slice_end);
                     references.push({
                         'begin': i,
-                        'template': getDirectiveTemplate(d, text, offset, this.mentions, this.options),
+                        'template': getDirectiveTemplate(d, text, offset, this.options),
                         end
                     });
                     i = end;

+ 7 - 7
src/shared/styling.js

@@ -24,12 +24,12 @@ const dont_escape = ['_', '>', '`', '~'];
 const styling_templates = {
     // m is the chatbox model
     // i is the offset of this directive relative to the start of the original message
-    'emphasis': (txt, i, mentions, options) => html`<span class="styling-directive">_</span><i>${renderStylingDirectiveBody(txt, i, mentions, options)}</i><span class="styling-directive">_</span>`,
+    'emphasis': (txt, i, options) => html`<span class="styling-directive">_</span><i>${renderStylingDirectiveBody(txt, i, options)}</i><span class="styling-directive">_</span>`,
     'preformatted': txt => html`<span class="styling-directive">\`</span><code>${txt}</code><span class="styling-directive">\`</span>`,
     'preformatted_block': txt => html`<div class="styling-directive">\`\`\`</div><code class="block">${txt}</code><div class="styling-directive">\`\`\`</div>`,
-    'quote': (txt, i, mentions, options) => html`<blockquote>${renderStylingDirectiveBody(txt, i, mentions, options)}</blockquote>`,
-    'strike': (txt, i, mentions, options) => html`<span class="styling-directive">~</span><del>${renderStylingDirectiveBody(txt, i, mentions, options)}</del><span class="styling-directive">~</span>`,
-    'strong': (txt, i, mentions, options) => html`<span class="styling-directive">*</span><b>${renderStylingDirectiveBody(txt, i, mentions, options)}</b><span class="styling-directive">*</span>`,
+    'quote': (txt, i, options) => html`<blockquote>${renderStylingDirectiveBody(txt, i, options)}</blockquote>`,
+    'strike': (txt, i, options) => html`<span class="styling-directive">~</span><del>${renderStylingDirectiveBody(txt, i, options)}</del><span class="styling-directive">~</span>`,
+    'strong': (txt, i, options) => html`<span class="styling-directive">*</span><b>${renderStylingDirectiveBody(txt, i, options)}</b><span class="styling-directive">*</span>`,
 };
 
 
@@ -141,15 +141,15 @@ export function getDirectiveAndLength (text, i) {
 export const isQuoteDirective = (d) => ['>', '&gt;'].includes(d);
 
 
-export function getDirectiveTemplate (d, text, offset, mentions, options) {
+export function getDirectiveTemplate (d, text, offset, options) {
     const template = styling_templates[styling_map[d].name];
     if (isQuoteDirective(d)) {
         const newtext = text
             .replace(/\n>/g, '\n') // Don't show the directive itself
             .replace(/\n$/, ''); // Trim line-break at the end
-        return template(newtext, offset, mentions, options);
+        return template(newtext, offset, options);
     } else {
-        return template(text, offset, mentions, options);
+        return template(text, offset, options);
     }
 }