From 28d374c617ba1f18001a2b1d57f9afd9471bb6ec Mon Sep 17 00:00:00 2001 From: Merci Jacob Date: Sat, 28 Feb 2026 18:27:37 +0200 Subject: [PATCH 1/4] feat: Allow pinning conversations to the top (XEP-0469) --- CHANGES.md | 1 + src/headless/plugins/bookmarks/collection.js | 45 +++- src/headless/plugins/bookmarks/model.js | 5 + src/headless/plugins/bookmarks/plugin.js | 1 + .../plugins/bookmarks/tests/bookmarks.js | 214 +++++++++++++++--- .../plugins/bookmarks/tests/deprecated.js | 4 +- src/headless/plugins/chat/model.js | 6 +- src/headless/plugins/headlines/feed.js | 1 - src/headless/plugins/muc/muc.js | 4 +- src/headless/shared/chatbox.js | 3 +- src/headless/shared/model-with-bookmark.js | 19 ++ src/headless/tests/mock.js | 6 +- .../types/plugins/bookmarks/collection.d.ts | 10 + .../types/plugins/bookmarks/model.d.ts | 1 + src/headless/types/plugins/chat/model.d.ts | 73 +++++- .../types/plugins/headlines/feed.d.ts | 1 - src/headless/types/plugins/muc/muc.d.ts | 73 +++++- src/headless/types/shared/chatbox.d.ts | 72 ++++++ .../types/shared/model-with-bookmark.d.ts | 78 +++++++ .../components/bookmarks-pin-list.js | 37 +++ .../components/styles/pin-list.scss | 19 ++ .../components/templates/pin-list.js | 38 ++++ src/plugins/bookmark-views/index.js | 2 + .../tests/bookmarks-pin-list.js | 58 +++++ src/plugins/bookmark-views/tests/bookmarks.js | 30 +-- src/plugins/controlbox/model.js | 1 - .../controlbox/templates/controlbox.js | 1 + src/plugins/roomslist/templates/roomslist.js | 75 +----- src/plugins/roomslist/tests/grouplists.js | 2 +- src/plugins/roomslist/view.js | 36 ++- src/shared/roomslist/templates/room-item.js | 116 ++++++++++ .../components/bookmarks-pin-list.d.ts | 5 + .../components/templates/pin-list.d.ts | 3 + src/types/plugins/controlbox/model.d.ts | 1 - src/types/plugins/roomslist/view.d.ts | 5 + .../shared/roomslist/templates/room-item.d.ts | 9 + 36 files changed, 916 insertions(+), 139 deletions(-) create mode 100644 src/headless/shared/model-with-bookmark.js create mode 100644 src/headless/types/shared/model-with-bookmark.d.ts create mode 100644 src/plugins/bookmark-views/components/bookmarks-pin-list.js create mode 100644 src/plugins/bookmark-views/components/styles/pin-list.scss create mode 100644 src/plugins/bookmark-views/components/templates/pin-list.js create mode 100644 src/plugins/bookmark-views/tests/bookmarks-pin-list.js create mode 100644 src/shared/roomslist/templates/room-item.js create mode 100644 src/types/plugins/bookmark-views/components/bookmarks-pin-list.d.ts create mode 100644 src/types/plugins/bookmark-views/components/templates/pin-list.d.ts create mode 100644 src/types/shared/roomslist/templates/room-item.d.ts diff --git a/CHANGES.md b/CHANGES.md index 94c9da8ea1..c77f2193ce 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -67,6 +67,7 @@ - #3941: add adhoc completed command result and text-multi as merged lines of text - #3989: properly display occupant hats in details dialog - Don't render unfurls for retracted messages. +- #3949: Allow pinning bookmarked conversations to the top (XEP-0469) ## 12.0.0 (2025-08-28) diff --git a/src/headless/plugins/bookmarks/collection.js b/src/headless/plugins/bookmarks/collection.js index 3c5f4e86f9..99da3dd36c 100644 --- a/src/headless/plugins/bookmarks/collection.js +++ b/src/headless/plugins/bookmarks/collection.js @@ -82,6 +82,7 @@ class Bookmarks extends Collection { const groupchat = await api.rooms.create(bookmark.get('jid'), { nick: bookmark.get('nick'), password: bookmark.get('password'), + pinned: bookmark.get('pinned'), }); groupchat.maybeShow(); } @@ -250,7 +251,7 @@ class Bookmarks extends Collection { markRoomAsBookmarked(bookmark) { const { chatboxes } = _converse.state; const groupchat = chatboxes.get(bookmark.get('jid')); - groupchat?.save('bookmarked', true); + groupchat?.setBookmark(bookmark); } /** @@ -334,6 +335,48 @@ class Bookmarks extends Collection { const { chatboxes } = _converse.state; return this.filter((b) => !chatboxes.get(b.get('jid'))); } + + /** + * + * @param {Bookmark} bookmark + */ + pinBookmark(bookmark) { + const extensions = [...bookmark.get('extensions'), ``]; + + bookmark.set('pinned', true); + + try { + api.bookmarks.set({ + jid: bookmark.get('jid'), + extensions, + }); + } catch (error) { + bookmark.set('pinned', false); + log.error('Error while trying to pin bookmark'); + log.error(error); + } + } + + /** + * + * @param {Bookmark} bookmark + */ + unpinBookmark(bookmark) { + const extensions = bookmark.get('extensions').filter(/** @param {String} e */ e => !(e.includes(' e.includes(' - IQ_stanzas.filter((s) => sizzle('iq publish[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`iq publish[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); expect(sent_stanza).toEqualStanza( @@ -27,9 +27,9 @@ describe('A bookmark', function () { type="set" xmlns="jabber:client"> - + - + ${nick} ${settings.password} @@ -65,7 +65,7 @@ describe('A bookmark', function () { id="${sent_stanza.getAttribute('id')}"/>`; _converse.api.connection.get()._dataRecv(mock.createRequest(_converse, stanza)); - expect(muc.get('bookmarked')).toBeTruthy(); + expect(muc.bookmark).toBeTruthy(); }), ); @@ -82,7 +82,7 @@ describe('A bookmark', function () { const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; let sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle('iq publish[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`iq publish[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); const stanza = stx` muc.get('nick') === newnick); _converse.api.connection.get()._dataRecv( - mock.createRequest(_converse, + mock.createRequest( + _converse, stx` - IQ_stanzas.filter((s) => sizzle('iq publish[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`iq publish[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); expect(sent_stanza).toEqualStanza( @@ -150,9 +152,9 @@ describe('A bookmark', function () { type="set" xmlns="jabber:client"> - + - + ${newnick} ${settings.password} @@ -194,7 +196,7 @@ describe('A bookmark', function () { const { bookmarks } = _converse.state; - let jid = 'theplay@conference.shakespeare.lit'; + const jid = 'theplay@conference.shakespeare.lit'; const model = bookmarks.create({ jid, autojoin: false, @@ -246,7 +248,7 @@ describe('A bookmark', function () { const { bookmarks } = _converse.state; await u.waitUntil(() => bookmarks.length); - await u.waitUntil(() => muc.get('bookmarked')); + await u.waitUntil(() => muc.bookmark); spyOn(bookmarks, 'sendBookmarkStanza').and.callThrough(); const sent_IQs = _converse.api.connection.get().IQ_stanzas; @@ -268,9 +270,9 @@ describe('A bookmark', function () { type="set" xmlns="jabber:client"> - + - + ${nick} @@ -320,15 +322,15 @@ describe('A bookmark', function () { const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; let sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle('publish[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); expect(sent_stanza).toEqualStanza(stx` - + - + @@ -363,16 +365,16 @@ describe('A bookmark', function () { sent_stanza = await u.waitUntil(() => IQ_stanzas.filter( - (s) => sizzle('publish[node="urn:xmpp:bookmarks:1"] conference[name="Balcony"]', s).length, + (s) => sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"] conference[name="Balcony"]`, s).length, ).pop(), ); expect(sent_stanza).toEqualStanza(stx` - + - + romeo @@ -414,16 +416,16 @@ describe('A bookmark', function () { sent_stanza = await u.waitUntil(() => IQ_stanzas.filter( - (s) => sizzle('publish[node="urn:xmpp:bookmarks:1"] conference[name="Garden"]', s).length, + (s) => sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"] conference[name="Garden"]`, s).length, ).pop(), ); expect(sent_stanza).toEqualStanza(stx` - + - + r0meo secret @@ -470,7 +472,7 @@ describe('A bookmark', function () { const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; const sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle(`items[node="urn:xmpp:bookmarks:1"]`, s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`items[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); // Simulate server response with item-not-found error @@ -480,7 +482,7 @@ describe('A bookmark', function () { from="${sent_stanza.getAttribute('to')}" to="${sent_stanza.getAttribute('from')}"> - + @@ -522,7 +524,7 @@ describe('A bookmark', function () { const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; let sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle('publish[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); // Server acknowledges successful storage @@ -543,17 +545,171 @@ describe('A bookmark', function () { // Check that a retract stanza is sent as per XEP-0402 sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle('retract[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`retract[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); expect(sent_stanza).toEqualStanza(stx` - + `); }), ); + + it( + 'can be pinned and sends out a stanza', + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + + const bare_jid = _converse.session.get('bare_jid'); + const muc_jid = 'theplay@conference.shakespeare.lit'; + const { api, state } = _converse; + + // First create a bookmark + state.bookmarks.create({ + jid: muc_jid, + autojoin: true, + name: 'The Play', + nick: 'romeo', + extensions: [], + }); + + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + await u.waitUntil(() => state.chatboxes.length === 1); + + const IQ_stanzas = api.connection.get().IQ_stanzas; + + // Now pin the bookmark + const bookmark = state.bookmarks.findWhere({ jid: muc_jid }); + expect(bookmark).toBeTruthy(); + await state.bookmarks.pinBookmark(bookmark); + + const sent_stanza = await u.waitUntil(() => + IQ_stanzas.filter( + (s) => + sizzle( + `publish[node="${Strophe.NS.BOOKMARKS2}"] conference[name="The Play"] extensions pinned`, + s, + ).length, + ).pop(), + ); + + expect(bookmark.get('pinned')).toBe(true); + + const chatbox = state.chatboxes.get(muc_jid); + expect(chatbox.bookmark.get('pinned')).toBe(true); + + expect(sent_stanza).toEqualStanza(stx` + + + + + + romeo + + + + + + + + + + http://jabber.org/protocol/pubsub#publish-options + + + true + + + max + + + never + + + whitelist + + + + + `); + }), + ); + + it( + 'can be unpinned and sends out a stanza', + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + + const bare_jid = _converse.session.get('bare_jid'); + const muc_jid = 'theplay@conference.shakespeare.lit'; + const { api, state } = _converse; + + // First create a pinned bookmark + const bookmark = state.bookmarks.create({ + jid: muc_jid, + autojoin: true, + name: 'The Play', + nick: 'romeo', + extensions: [``], + }); + + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + await u.waitUntil(() => state.chatboxes.length === 1); + + const IQ_stanzas = api.connection.get().IQ_stanzas; + + expect(bookmark.get('pinned')).toBe(true); + expect(state.chatboxes.get(muc_jid).bookmark.get('pinned')).toBe(true); + + // Now unpin the bookmark + await state.bookmarks.unpinBookmark(bookmark); + + const sent_stanza = await u.waitUntil(() => + IQ_stanzas.filter((s) => { + return sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"] conference[name="The Play"]`, s).length; + }).pop(), + ); + + expect(bookmark.get('pinned')).toBe(false); + expect(state.chatboxes.get(muc_jid).bookmark.get('pinned')).toBe(false); + + expect(sent_stanza).toEqualStanza(stx` + + + + + + romeo + + + + + + + http://jabber.org/protocol/pubsub#publish-options + + + true + + + max + + + never + + + whitelist + + + + + `); + }), + ); }); diff --git a/src/headless/plugins/bookmarks/tests/deprecated.js b/src/headless/plugins/bookmarks/tests/deprecated.js index e995fd3b39..9188eb2357 100644 --- a/src/headless/plugins/bookmarks/tests/deprecated.js +++ b/src/headless/plugins/bookmarks/tests/deprecated.js @@ -78,7 +78,7 @@ describe('A chat room', function () { id="${sent_stanza.getAttribute('id')}"/>`; _converse.api.connection.get()._dataRecv(mock.createRequest(_converse, stanza)); - expect(muc.get('bookmarked')).toBeTruthy(); + expect(muc.bookmark).toBeTruthy(); }), ); }); @@ -106,7 +106,7 @@ describe('A bookmark', function () { const { bookmarks } = _converse.state; await u.waitUntil(() => bookmarks.length); - await u.waitUntil(() => muc.get('bookmarked')); + await u.waitUntil(() => muc.bookmark); spyOn(bookmarks, 'sendBookmarkStanza').and.callThrough(); const sent_IQs = _converse.api.connection.get().IQ_stanzas; diff --git a/src/headless/plugins/chat/model.js b/src/headless/plugins/chat/model.js index acda48fcb9..bcaf58ffc8 100644 --- a/src/headless/plugins/chat/model.js +++ b/src/headless/plugins/chat/model.js @@ -6,6 +6,7 @@ import converse from '../../shared/api/public.js'; import log from '@converse/log'; import { isUniView } from '../../utils/session.js'; import { sendChatState, sendMarker } from '../../shared/actions.js'; +import ModelWithBookmark from '../../shared/model-with-bookmark.js'; import ModelWithMessages from '../../shared/model-with-messages.js'; import ModelWithVCard from '../../shared/model-with-vcard.js'; import ModelWithContact from '../../shared/model-with-contact.js'; @@ -17,7 +18,9 @@ const { Strophe, u } = converse.env; /** * Represents a one-on-one chat conversation. */ -class ChatBox extends ModelWithVCard(ModelWithMessages(ModelWithContact(ColorAwareModel(ChatBoxBase)))) { +class ChatBox extends ModelWithBookmark( + ModelWithVCard(ModelWithMessages(ModelWithContact(ColorAwareModel(ChatBoxBase)))) +) { /** * @typedef {import('./message.js').default} Message * @typedef {import('../muc/muc.js').default} MUC @@ -27,7 +30,6 @@ class ChatBox extends ModelWithVCard(ModelWithMessages(ModelWithContact(ColorAwa defaults() { return { - bookmarked: false, hidden: isUniView() && !api.settings.get('singleton'), message_type: 'chat', num_unread: 0, diff --git a/src/headless/plugins/headlines/feed.js b/src/headless/plugins/headlines/feed.js index 6866c38138..1dd9f3d3e6 100644 --- a/src/headless/plugins/headlines/feed.js +++ b/src/headless/plugins/headlines/feed.js @@ -9,7 +9,6 @@ import ChatBoxBase from '../../shared/chatbox.js'; export default class HeadlinesFeed extends ChatBoxBase { defaults() { return { - 'bookmarked': false, 'hidden': isUniView() && !api.settings.get('singleton'), 'message_type': 'headline', 'num_unread': 0, diff --git a/src/headless/plugins/muc/muc.js b/src/headless/plugins/muc/muc.js index bf7b2c098e..0bb4eb0b1d 100644 --- a/src/headless/plugins/muc/muc.js +++ b/src/headless/plugins/muc/muc.js @@ -44,6 +44,7 @@ import { sendMarker } from '../../shared/actions.js'; import ChatBoxBase from '../../shared/chatbox.js'; import ColorAwareModel from '../../shared/color.js'; import ModelWithMessages from '../../shared/model-with-messages.js'; +import ModelWithBookmark from '../../shared/model-with-bookmark.js'; import ModelWithVCard from '../../shared/model-with-vcard.js'; import { shouldCreateGroupchatMessage, isInfoVisible } from './utils.js'; import MUCSession from './session.js'; @@ -55,7 +56,7 @@ const DISCO_INFO_TIMEOUT_ON_JOIN = 30000; /** * Represents a groupchat conversation. */ -class MUC extends ModelWithVCard(ModelWithMessages(ColorAwareModel(ChatBoxBase))) { +class MUC extends ModelWithBookmark(ModelWithVCard(ModelWithMessages(ColorAwareModel(ChatBoxBase)))) { /** * @typedef {import('../../shared/message.js').default} BaseMessage * @typedef {import('./message.js').default} MUCMessage @@ -74,7 +75,6 @@ class MUC extends ModelWithVCard(ModelWithMessages(ColorAwareModel(ChatBoxBase)) defaults() { /** @type {import('./types').DefaultMUCAttributes} */ return { - bookmarked: false, chat_state: undefined, closed: false, has_activity: false, // XEP-437 diff --git a/src/headless/shared/chatbox.js b/src/headless/shared/chatbox.js index a4f4a4a612..a87f5bfe82 100644 --- a/src/headless/shared/chatbox.js +++ b/src/headless/shared/chatbox.js @@ -5,13 +5,14 @@ import _converse from './_converse.js'; import converse from './api/public.js'; import log from '@converse/log'; import ModelWithMessages from './model-with-messages.js'; +import ModelWithBookmark from './model-with-bookmark.js'; const { u } = converse.env; /** * Base class for all chat boxes. Provides common methods. */ -export default class ChatBoxBase extends ModelWithMessages(Model) { +export default class ChatBoxBase extends ModelWithBookmark(ModelWithMessages(Model)) { async initialize() { await super.initialize(); const jid = this.get('jid'); diff --git a/src/headless/shared/model-with-bookmark.js b/src/headless/shared/model-with-bookmark.js new file mode 100644 index 0000000000..0c226e07a4 --- /dev/null +++ b/src/headless/shared/model-with-bookmark.js @@ -0,0 +1,19 @@ + +/** + * @template {import('./types').ModelExtender} T + * @param {T} BaseModel + */ +export default function ModelWithBookmark(BaseModel) { + return class ModelWithBookmark extends BaseModel { + initialize() { + super.initialize(); + this.bookmark = null; + } + + setBookmark(bookmark) { + this.bookmark = bookmark; + this.listenTo(this.bookmark, 'change', () => this.trigger('bookmark:change', bookmark)); + this.trigger('bookmark:change', bookmark); + } + }; +} diff --git a/src/headless/tests/mock.js b/src/headless/tests/mock.js index c8d242b694..5175b27a26 100644 --- a/src/headless/tests/mock.js +++ b/src/headless/tests/mock.js @@ -223,11 +223,11 @@ export async function waitUntilBookmarksReturned( id="${sent_stanza.getAttribute('id')}" xmlns="jabber:client"> - + ${bookmarks.map( (b) => stx` - ${b.nick ? stx`${b.nick}` : ''} @@ -459,7 +459,7 @@ export async function openAndEnterMUC( await room_creation_promise; const model = _converse.chatboxes.get(muc_jid); - await u.waitUntil(() => model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED); + await u.waitUntil(() => model.session.get('connection_status') === window.converse.ROOMSTATUS.ENTERED); const affs = api.settings.get('muc_fetch_members'); const all_affiliations = Array.isArray(affs) ? affs : affs ? ['member', 'admin', 'owner'] : []; diff --git a/src/headless/types/plugins/bookmarks/collection.d.ts b/src/headless/types/plugins/bookmarks/collection.d.ts index f4ec17cfeb..4ffdca2a06 100644 --- a/src/headless/types/plugins/bookmarks/collection.d.ts +++ b/src/headless/types/plugins/bookmarks/collection.d.ts @@ -78,6 +78,16 @@ declare class Bookmarks extends Collection { */ onBookmarksReceivedError(deferred: any, iq: Element): Promise; getUnopenedBookmarks(): Promise; + /** + * + * @param {Bookmark} bookmark + */ + pinBookmark(bookmark: Bookmark): void; + /** + * + * @param {Bookmark} bookmark + */ + unpinBookmark(bookmark: Bookmark): void; } import Bookmark from './model.js'; import { Collection } from '@converse/skeletor'; diff --git a/src/headless/types/plugins/bookmarks/model.d.ts b/src/headless/types/plugins/bookmarks/model.d.ts index 0ba37472ea..4b62200ecb 100644 --- a/src/headless/types/plugins/bookmarks/model.d.ts +++ b/src/headless/types/plugins/bookmarks/model.d.ts @@ -1,6 +1,7 @@ export default Bookmark; declare class Bookmark extends Model { constructor(attributes?: Partial, options?: import("@converse/skeletor").ModelOptions); + initialize(): void; getDisplayName(): any; } import { Model } from '@converse/skeletor'; diff --git a/src/headless/types/plugins/chat/model.d.ts b/src/headless/types/plugins/chat/model.d.ts index 4d928c0d7d..2d9ee492f3 100644 --- a/src/headless/types/plugins/chat/model.d.ts +++ b/src/headless/types/plugins/chat/model.d.ts @@ -1,5 +1,77 @@ export default ChatBox; declare const ChatBox_base: { + new (...args: any[]): { + [x: string]: any; + initialize(): void; + bookmark: any; + setBookmark(bookmark: any): void; + _browserStorage?: import("@converse/skeletor").BrowserStorage; + _changing: boolean; + _pending: boolean | import("@converse/skeletor").ModelOptions; + _previousAttributes?: import("@converse/skeletor").ModelAttributes; + _url: string; + _urlRoot: string; + attributes: import("@converse/skeletor").ModelAttributes; + changed: Partial; + cid: string; + collection?: import("@converse/skeletor").Collection; + id: string | number; + validationError: string | number | null; + browserStorage: import("@converse/skeletor").BrowserStorage; + readonly idAttribute: string; + readonly cidPrefix: string; + preinitialize(...args: any[]): void; + validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; + defaults(): Partial; + toJSON(): import("@converse/skeletor").ModelAttributes; + sync(method: import("@converse/skeletor").SyncOperation, model: import("@converse/skeletor").Model, options: import("@converse/skeletor").Options): any; + get(attr: K): import("@converse/skeletor").ModelAttributes[K]; + keys(): string[]; + values(): any[]; + pairs(): [string | number, any][]; + entries(): [string | number, any][]; + invert(): Record; + pick(...args: K[]): Pick; + omit(...args: K[]): Omit; + isEmpty(): boolean; + has(attr: string | number): boolean; + matches(attrs: Partial): boolean; + set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; + clear(options?: import("@converse/skeletor").ModelOptions): any; + hasChanged(attr?: string | number): boolean; + changedAttributes(diff?: Partial): false | Partial; + previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; + previousAttributes(): import("@converse/skeletor").ModelAttributes; + fetch(options?: import("@converse/skeletor").Options): any; + save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + destroy(options?: import("@converse/skeletor").ModelOptions): any; + urlRoot: string; + url: string; + parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; + isNew(): boolean; + isValid(options?: import("@converse/skeletor").ModelOptions): boolean; + _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; + _events?: import("@converse/skeletor").EventHandlersMap; + _listeners?: import("@converse/skeletor").EventListenerMap; + _listeningTo?: import("@converse/skeletor").EventListenerMap; + _listenId?: string; + on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; + stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + trigger(name: string, ...args: any[]): any; + constructor: Function; + toString(): string; + toLocaleString(): string; + valueOf(): Object; + hasOwnProperty(v: PropertyKey): boolean; + isPrototypeOf(v: Object): boolean; + propertyIsEnumerable(v: PropertyKey): boolean; + }; +} & { new (...args: any[]): { [x: string]: any; _vcard: import("../vcard/vcard.js").default; @@ -382,7 +454,6 @@ declare class ChatBox extends ChatBox_base { * @typedef {import('../../shared/errors').StanzaParseError} StanzaParseError */ defaults(): { - bookmarked: boolean; hidden: boolean; message_type: string; num_unread: number; diff --git a/src/headless/types/plugins/headlines/feed.d.ts b/src/headless/types/plugins/headlines/feed.d.ts index 6e84b65f03..f52ba480ae 100644 --- a/src/headless/types/plugins/headlines/feed.d.ts +++ b/src/headless/types/plugins/headlines/feed.d.ts @@ -8,7 +8,6 @@ export default class HeadlinesFeed extends ChatBoxBase { */ constructor(attrs: import("@converse/skeletor").ModelAttributes, options: import("@converse/skeletor").ModelOptions); defaults(): { - bookmarked: boolean; hidden: boolean; message_type: string; num_unread: number; diff --git a/src/headless/types/plugins/muc/muc.d.ts b/src/headless/types/plugins/muc/muc.d.ts index 65be76f7ae..7c054e60c1 100644 --- a/src/headless/types/plugins/muc/muc.d.ts +++ b/src/headless/types/plugins/muc/muc.d.ts @@ -1,5 +1,77 @@ export default MUC; declare const MUC_base: { + new (...args: any[]): { + [x: string]: any; + initialize(): void; + bookmark: any; + setBookmark(bookmark: any): void; + _browserStorage?: import("@converse/skeletor").BrowserStorage; + _changing: boolean; + _pending: boolean | import("@converse/skeletor").ModelOptions; + _previousAttributes?: import("@converse/skeletor").ModelAttributes; + _url: string; + _urlRoot: string; + attributes: import("@converse/skeletor").ModelAttributes; + changed: Partial; + cid: string; + collection?: import("@converse/skeletor").Collection; + id: string | number; + validationError: string | number | null; + browserStorage: import("@converse/skeletor").BrowserStorage; + readonly idAttribute: string; + readonly cidPrefix: string; + preinitialize(...args: any[]): void; + validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; + defaults(): Partial; + toJSON(): import("@converse/skeletor").ModelAttributes; + sync(method: import("@converse/skeletor").SyncOperation, model: Model, options: import("@converse/skeletor").Options): any; + get(attr: K): import("@converse/skeletor").ModelAttributes[K]; + keys(): string[]; + values(): any[]; + pairs(): [string | number, any][]; + entries(): [string | number, any][]; + invert(): Record; + pick(...args: K[]): Pick; + omit(...args: K[]): Omit; + isEmpty(): boolean; + has(attr: string | number): boolean; + matches(attrs: Partial): boolean; + set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; + clear(options?: import("@converse/skeletor").ModelOptions): any; + hasChanged(attr?: string | number): boolean; + changedAttributes(diff?: Partial): false | Partial; + previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; + previousAttributes(): import("@converse/skeletor").ModelAttributes; + fetch(options?: import("@converse/skeletor").Options): any; + save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + destroy(options?: import("@converse/skeletor").ModelOptions): any; + urlRoot: string; + url: string; + parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; + isNew(): boolean; + isValid(options?: import("@converse/skeletor").ModelOptions): boolean; + _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; + _events?: import("@converse/skeletor").EventHandlersMap; + _listeners?: import("@converse/skeletor").EventListenerMap; + _listeningTo?: import("@converse/skeletor").EventListenerMap; + _listenId?: string; + on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; + stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + trigger(name: string, ...args: any[]): any; + constructor: Function; + toString(): string; + toLocaleString(): string; + valueOf(): Object; + hasOwnProperty(v: PropertyKey): boolean; + isPrototypeOf(v: Object): boolean; + propertyIsEnumerable(v: PropertyKey): boolean; + }; +} & { new (...args: any[]): { [x: string]: any; _vcard: import("../vcard/vcard.js").default; @@ -305,7 +377,6 @@ declare class MUC extends MUC_base { * @typedef {import('../../shared/errors').StanzaParseError} StanzaParseError */ defaults(): { - bookmarked: boolean; chat_state: any; closed: boolean; has_activity: boolean; diff --git a/src/headless/types/shared/chatbox.d.ts b/src/headless/types/shared/chatbox.d.ts index 3ffdce8f92..2baa9307fa 100644 --- a/src/headless/types/shared/chatbox.d.ts +++ b/src/headless/types/shared/chatbox.d.ts @@ -1,4 +1,76 @@ declare const ChatBoxBase_base: { + new (...args: any[]): { + [x: string]: any; + initialize(): void; + bookmark: any; + setBookmark(bookmark: any): void; + _browserStorage?: import("@converse/skeletor").BrowserStorage; + _changing: boolean; + _pending: boolean | import("@converse/skeletor").ModelOptions; + _previousAttributes?: import("@converse/skeletor").ModelAttributes; + _url: string; + _urlRoot: string; + attributes: import("@converse/skeletor").ModelAttributes; + changed: Partial; + cid: string; + collection?: import("@converse/skeletor").Collection; + id: string | number; + validationError: string | number | null; + browserStorage: import("@converse/skeletor").BrowserStorage; + readonly idAttribute: string; + readonly cidPrefix: string; + preinitialize(...args: any[]): void; + validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; + defaults(): Partial; + toJSON(): import("@converse/skeletor").ModelAttributes; + sync(method: import("@converse/skeletor").SyncOperation, model: Model, options: import("@converse/skeletor").Options): any; + get(attr: K): import("@converse/skeletor").ModelAttributes[K]; + keys(): string[]; + values(): any[]; + pairs(): [string | number, any][]; + entries(): [string | number, any][]; + invert(): Record; + pick(...args: K[]): Pick; + omit(...args: K[]): Omit; + isEmpty(): boolean; + has(attr: string | number): boolean; + matches(attrs: Partial): boolean; + set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; + clear(options?: import("@converse/skeletor").ModelOptions): any; + hasChanged(attr?: string | number): boolean; + changedAttributes(diff?: Partial): false | Partial; + previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; + previousAttributes(): import("@converse/skeletor").ModelAttributes; + fetch(options?: import("@converse/skeletor").Options): any; + save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + destroy(options?: import("@converse/skeletor").ModelOptions): any; + urlRoot: string; + url: string; + parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; + isNew(): boolean; + isValid(options?: import("@converse/skeletor").ModelOptions): boolean; + _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; + _events?: import("@converse/skeletor").EventHandlersMap; + _listeners?: import("@converse/skeletor").EventListenerMap; + _listeningTo?: import("@converse/skeletor").EventListenerMap; + _listenId?: string; + on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; + stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + trigger(name: string, ...args: any[]): any; + constructor: Function; + toString(): string; + toLocaleString(): string; + valueOf(): Object; + hasOwnProperty(v: PropertyKey): boolean; + isPrototypeOf(v: Object): boolean; + propertyIsEnumerable(v: PropertyKey): boolean; + }; +} & { new (...args: any[]): { [x: string]: any; disable_mam: boolean; diff --git a/src/headless/types/shared/model-with-bookmark.d.ts b/src/headless/types/shared/model-with-bookmark.d.ts new file mode 100644 index 0000000000..3b9baf723e --- /dev/null +++ b/src/headless/types/shared/model-with-bookmark.d.ts @@ -0,0 +1,78 @@ +/** + * @template {import('./types').ModelExtender} T + * @param {T} BaseModel + */ +export default function ModelWithBookmark(BaseModel: T): { + new (...args: any[]): { + [x: string]: any; + initialize(): void; + bookmark: any; + setBookmark(bookmark: any): void; + _browserStorage?: import("@converse/skeletor").BrowserStorage; + _changing: boolean; + _pending: boolean | import("@converse/skeletor").ModelOptions; + _previousAttributes?: import("@converse/skeletor").ModelAttributes; + _url: string; + _urlRoot: string; + attributes: import("@converse/skeletor").ModelAttributes; + changed: Partial; + cid: string; + collection?: import("@converse/skeletor").Collection; + id: string | number; + validationError: string | number | null; + browserStorage: import("@converse/skeletor").BrowserStorage; + readonly idAttribute: string; + readonly cidPrefix: string; + preinitialize(...args: any[]): void; + validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; + defaults(): Partial; + toJSON(): import("@converse/skeletor").ModelAttributes; + sync(method: import("@converse/skeletor").SyncOperation, model: import("@converse/skeletor").Model, options: import("@converse/skeletor").Options): any; + get(attr: K): import("@converse/skeletor").ModelAttributes[K]; + keys(): string[]; + values(): any[]; + pairs(): [string | number, any][]; + entries(): [string | number, any][]; + invert(): Record; + pick(...args: K[]): Pick; + omit(...args: K[]): Omit; + isEmpty(): boolean; + has(attr: string | number): boolean; + matches(attrs: Partial): boolean; + set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; + clear(options?: import("@converse/skeletor").ModelOptions): any; + hasChanged(attr?: string | number): boolean; + changedAttributes(diff?: Partial): false | Partial; + previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; + previousAttributes(): import("@converse/skeletor").ModelAttributes; + fetch(options?: import("@converse/skeletor").Options): any; + save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; + destroy(options?: import("@converse/skeletor").ModelOptions): any; + urlRoot: string; + url: string; + parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; + isNew(): boolean; + isValid(options?: import("@converse/skeletor").ModelOptions): boolean; + _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; + _events?: import("@converse/skeletor").EventHandlersMap; + _listeners?: import("@converse/skeletor").EventListenerMap; + _listeningTo?: import("@converse/skeletor").EventListenerMap; + _listenId?: string; + on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; + stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; + listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; + trigger(name: string, ...args: any[]): any; + constructor: Function; + toString(): string; + toLocaleString(): string; + valueOf(): Object; + hasOwnProperty(v: PropertyKey): boolean; + isPrototypeOf(v: Object): boolean; + propertyIsEnumerable(v: PropertyKey): boolean; + }; +} & T; +//# sourceMappingURL=model-with-bookmark.d.ts.map \ No newline at end of file diff --git a/src/plugins/bookmark-views/components/bookmarks-pin-list.js b/src/plugins/bookmark-views/components/bookmarks-pin-list.js new file mode 100644 index 0000000000..6574b58278 --- /dev/null +++ b/src/plugins/bookmark-views/components/bookmarks-pin-list.js @@ -0,0 +1,37 @@ +import { _converse, api, constants, Model, u } from '@converse/headless'; +import tplBookmarksPinList from './templates/pin-list.js'; +import { RoomsList } from 'plugins/roomslist/view.js'; + +const { initStorage } = u; + +export class PinnedBookmarksView extends RoomsList { + model = null; + + initialize() { + const bare_jid = _converse.session.get('bare_jid'); + const id = `converse.bookmarks-pin-list-model-${bare_jid}`; + this.model = new Model({ toggle_state: constants.OPENED }); + _converse.state.bookmarks_pin_list = this.model; + + initStorage(this.model, id); + this.model.fetch(); + + this.addEventListeners(); + + this.requestUpdate(); + } + + /** @returns {import('@converse/headless').MUC[]} */ + getRoomsToShow() { + const { chatboxes } = _converse.state; + const rooms = chatboxes.filter((m) => m.bookmark?.get('pinned')); + rooms.sort((a, b) => (a.getDisplayName().toLowerCase() <= b.getDisplayName().toLowerCase() ? -1 : 1)); + return rooms; + } + + render() { + return tplBookmarksPinList(this); + } +} + +api.elements.define('converse-pinned-bookmarks', PinnedBookmarksView); diff --git a/src/plugins/bookmark-views/components/styles/pin-list.scss b/src/plugins/bookmark-views/components/styles/pin-list.scss new file mode 100644 index 0000000000..6abb76e75d --- /dev/null +++ b/src/plugins/bookmark-views/components/styles/pin-list.scss @@ -0,0 +1,19 @@ +.conversejs { + converse-pinned-bookmarks { + padding-bottom: 1rem; + .list-item { + .open-room { + display: flex; + flex-direction: row; + line-height: 1.5em; + height: 2.5em; + padding: 0.2em 0; + span { + overflow-x: hidden; + text-overflow: ellipsis; + padding-top: 0.25em; + } + } + } + } +} diff --git a/src/plugins/bookmark-views/components/templates/pin-list.js b/src/plugins/bookmark-views/components/templates/pin-list.js new file mode 100644 index 0000000000..c2477ba1e0 --- /dev/null +++ b/src/plugins/bookmark-views/components/templates/pin-list.js @@ -0,0 +1,38 @@ +import { constants } from "@converse/headless"; +import { __ } from "i18n"; +import { html } from "lit"; +import { tplRoomItem } from "shared/roomslist/templates/room-item.js"; +import '../styles/pin-list.scss'; + +/** + * @param {import('plugins/bookmark-views/components/bookmarks-pin-list').PinnedBookmarksView} el + */ +export default (el) => { + const rooms = el.getRoomsToShow(); + const is_closed = el.model.get('toggle_state') === constants.CLOSED; + + return html` +
+ + el.toggleRoomsList(ev)}> + + ${__('Pinned groupchats')} + + ${rooms.length ? html`` : '' } + + +
+ +
+
    + ${ + rooms.map(/** @param {import('@converse/headless').MUC} room */(room) => tplRoomItem(el, room)) + } +
+
`; +} diff --git a/src/plugins/bookmark-views/index.js b/src/plugins/bookmark-views/index.js index e927fff4ea..82f8efcb9d 100644 --- a/src/plugins/bookmark-views/index.js +++ b/src/plugins/bookmark-views/index.js @@ -9,6 +9,7 @@ import BookmarkForm from './components/bookmark-form.js'; import BookmarksView from './components/bookmarks-list.js'; import { BookmarkableChatRoomView } from './mixins.js'; import { removeBookmarkViaEvent } from './utils.js'; +import { PinnedBookmarksView } from './components/bookmarks-pin-list.js'; import './styles/bookmarks.scss'; @@ -36,6 +37,7 @@ converse.plugins.add('converse-bookmark-views', { removeBookmarkViaEvent, MUCBookmarkForm: BookmarkForm, BookmarksView, + PinnedBookmarksView, }; Object.assign(_converse, exports); // DEPRECATED diff --git a/src/plugins/bookmark-views/tests/bookmarks-pin-list.js b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js new file mode 100644 index 0000000000..cec499e123 --- /dev/null +++ b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js @@ -0,0 +1,58 @@ +import mock from '../../../shared/tests/mock.js'; +import converse from '../../../../dist/converse.js'; + +const { Strophe, u } = converse.env; + +describe("The bookmarks pin list", function () { + it("shows a list of pinned bookmarks", mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + const { api } = _converse; + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + await mock.openControlBox(_converse); + + const bookmarks_pin_list = document.querySelector('converse-pinned-bookmarks'); + const main_list = document.querySelector('converse-rooms-list'); + + let muc_jid = 'room@conference.shakespeare.lit'; + await api.bookmarks.set({ + jid: muc_jid, + name: 'Romeo\'s room', + autojoin: true, + nick: 'romeo', + extensions: [``], + }) + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length); + let room_els = bookmarks_pin_list.querySelectorAll(".open-room"); + expect(room_els.length).toBe(1); + expect(main_list.querySelectorAll(".open-room").length).toBe(0); + + muc_jid = 'lounge@montague.lit'; + await api.bookmarks.set({ + jid: muc_jid, + name: 'Lounge', + autojoin: true, + nick: 'romeo', + extensions: [``], + }); + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length > 1); + room_els = bookmarks_pin_list.querySelectorAll(".open-room"); + expect(room_els.length).toBe(2); + expect(main_list.querySelectorAll(".open-room").length).toBe(0); + + // Unpin a room + bookmarks_pin_list.querySelector('.unpin-room').click(); + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length === 1); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(1); + expect(main_list.querySelectorAll(".open-room").length).toBe(1); + + // pin it again + main_list.querySelector('.pin-room').click(); + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length === 2); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(2); + expect(main_list.querySelectorAll(".open-room").length).toBe(0); + })); +}); diff --git a/src/plugins/bookmark-views/tests/bookmarks.js b/src/plugins/bookmark-views/tests/bookmarks.js index 752bb0f76f..e184123378 100644 --- a/src/plugins/bookmark-views/tests/bookmarks.js +++ b/src/plugins/bookmark-views/tests/bookmarks.js @@ -49,14 +49,14 @@ describe('Bookmarks', function () { - JC - JC @@ -80,19 +80,19 @@ describe('Bookmarks', function () { id="${u.getUniqueId()}" xmlns="jabber:client"> - + - + JC - + JC - + JC @@ -117,19 +117,19 @@ describe('Bookmarks', function () { id="${u.getUniqueId()}" xmlns="jabber:client"> - + - + JC - + JC - + JC @@ -164,13 +164,13 @@ describe('Bookmarks', function () { // Client requests all items const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; const sent_stanza = await u.waitUntil(() => - IQ_stanzas.filter((s) => sizzle('items[node="urn:xmpp:bookmarks:1"]', s).length).pop(), + IQ_stanzas.filter((s) => sizzle(`items[node="${Strophe.NS.BOOKMARKS2}"]`, s).length).pop(), ); expect(sent_stanza).toEqualStanza( stx` - + `, ); @@ -186,16 +186,16 @@ describe('Bookmarks', function () { to="${_converse.jid}" id="${sent_stanza.getAttribute('id')}"> - + - JC - diff --git a/src/plugins/controlbox/model.js b/src/plugins/controlbox/model.js index c23d5fb1ec..9e8f59ebf5 100644 --- a/src/plugins/controlbox/model.js +++ b/src/plugins/controlbox/model.js @@ -13,7 +13,6 @@ const { CONTROLBOX_TYPE } = constants; class ControlBox extends Model { defaults() { return { - bookmarked: false, box_id: 'controlbox', chat_state: undefined, closed: !api.settings.get('show_controlbox_by_default'), diff --git a/src/plugins/controlbox/templates/controlbox.js b/src/plugins/controlbox/templates/controlbox.js index de632966d9..d6887326c2 100644 --- a/src/plugins/controlbox/templates/controlbox.js +++ b/src/plugins/controlbox/templates/controlbox.js @@ -47,6 +47,7 @@ export default (el) => { ? html`
+
${api.settings.get('authentication') === ANONYMOUS ? '' diff --git a/src/plugins/roomslist/templates/roomslist.js b/src/plugins/roomslist/templates/roomslist.js index 2c3b98c28e..94d78507aa 100644 --- a/src/plugins/roomslist/templates/roomslist.js +++ b/src/plugins/roomslist/templates/roomslist.js @@ -3,85 +3,14 @@ * @typedef {import('@converse/headless').MUC} MUC */ import { html } from 'lit'; -import { api, u, constants } from '@converse/headless'; +import { api, constants } from '@converse/headless'; import 'plugins/muc-views/modals/add-muc.js'; import 'plugins/muc-views/modals/muc-list.js'; import { __ } from 'i18n'; -import { getUnreadMsgsDisplay } from 'shared/chat/utils.js'; - import '../styles/roomsgroups.scss'; +import { tplRoomItem } from 'shared/roomslist/templates/room-item.js'; const { CLOSED } = constants; -const { isUniView } = u; - -/** @param {MUC} room */ -function isCurrentlyOpen(room) { - return isUniView() && !room.get('hidden'); -} - -/** @param {MUC} room */ -function tplUnreadIndicator(room) { - return html`${getUnreadMsgsDisplay(room)}`; -} - -function tplActivityIndicator() { - return html``; -} - -/** - * @param {RoomsList} el - * @param {MUC} room - */ -function tplRoomItem(el, room) { - const i18n_leave_room = __('Leave this groupchat'); - const has_unread_msgs = room.get('num_unread_general') || room.get('has_activity'); - return html`
  • - el.openRoom(ev)} - > - - ${room.get('num_unread') - ? tplUnreadIndicator(room) - : room.get('has_activity') - ? tplActivityIndicator() - : ''} - ${room.getDisplayName()} - - - el.closeRoom(ev)} - > - - -
    • `; -} /** * @param {RoomsList} el diff --git a/src/plugins/roomslist/tests/grouplists.js b/src/plugins/roomslist/tests/grouplists.js index cf281e0d19..659a078cfe 100644 --- a/src/plugins/roomslist/tests/grouplists.js +++ b/src/plugins/roomslist/tests/grouplists.js @@ -108,7 +108,7 @@ describe('A MUC domain group', function () { await mock.waitForRoster(_converse, 'current', 0); await mock.openControlBox(_converse); const controlbox = _converse.chatboxviews.get('controlbox'); - const list = controlbox.querySelector('.list-container--openrooms'); + const list = controlbox.querySelector('converse-rooms-list .list-container--openrooms'); const nick = 'JC'; const muc_jid = 'room@conference.shakespeare.lit'; _converse.api.rooms.open(muc_jid, { nick }); diff --git a/src/plugins/roomslist/view.js b/src/plugins/roomslist/view.js index 1b14e515c6..79ee593bce 100644 --- a/src/plugins/roomslist/view.js +++ b/src/plugins/roomslist/view.js @@ -19,6 +19,12 @@ export class RoomsList extends CustomElement { initStorage(this.model, id); this.model.fetch(); + this.addEventListeners(); + + this.requestUpdate(); + } + + addEventListeners() { const { chatboxes } = _converse.state; this.listenTo(chatboxes, 'add', this.renderIfChatRoom); this.listenTo(chatboxes, 'remove', this.renderIfChatRoom); @@ -26,9 +32,8 @@ export class RoomsList extends CustomElement { this.listenTo(chatboxes, 'change', this.renderIfRelevantChange); this.listenTo(chatboxes, 'vcard:add', () => this.requestUpdate()); this.listenTo(chatboxes, 'vcard:change', () => this.requestUpdate()); + this.listenTo(chatboxes, 'bookmark:change', () => this.requestUpdate()); this.listenTo(this.model, 'change', () => this.requestUpdate()); - - this.requestUpdate(); } render() { @@ -42,7 +47,7 @@ export class RoomsList extends CustomElement { /** @param {import('@converse/headless').Model} model */ renderIfRelevantChange(model) { - const attrs = ['bookmarked', 'hidden', 'name', 'num_unread', 'num_unread_general', 'has_activity']; + const attrs = ['hidden', 'name', 'num_unread', 'num_unread_general', 'has_activity']; const changed = model.changed || {}; if (u.muc.isChatRoom(model) && Object.keys(changed).filter((m) => attrs.includes(m)).length) { this.requestUpdate(); @@ -52,7 +57,7 @@ export class RoomsList extends CustomElement { /** @returns {import('@converse/headless').MUC[]} */ getRoomsToShow() { const { chatboxes } = _converse.state; - const rooms = chatboxes.filter((m) => m.get('type') === CHATROOMS_TYPE && !m.get('closed')); + const rooms = chatboxes.filter((m) => m.get('type') === CHATROOMS_TYPE && !m.get('closed') && !m.bookmark?.get('pinned')); rooms.sort((a, b) => (a.getDisplayName().toLowerCase() <= b.getDisplayName().toLowerCase() ? -1 : 1)); return rooms; } @@ -82,6 +87,29 @@ export class RoomsList extends CustomElement { } } + /** @param {Event} ev */ + pinRoom(ev) { + ev.preventDefault(); + const target = /** @type {HTMLElement} */ (ev.currentTarget); + const jid = target.getAttribute('data-room-jid'); + const { bookmarks } = _converse.state; + bookmarks + .where({ jid }) + .forEach(/** @param {import('@converse/headless').Bookmark} b */ (b) => + bookmarks.pinBookmark(b)); + } + + /** @param {Event} ev */ + unpinRoom(ev) { + ev.preventDefault(); + const target = /** @type {HTMLElement} */ (ev.currentTarget); + const jid = target.getAttribute('data-room-jid'); + const { bookmarks } = _converse.state; + bookmarks + .where({ jid }) + .forEach((b) => bookmarks.unpinBookmark(b)); + } + /** @param {Event} [ev] */ toggleRoomsList(ev) { ev?.preventDefault?.(); diff --git a/src/shared/roomslist/templates/room-item.js b/src/shared/roomslist/templates/room-item.js new file mode 100644 index 0000000000..8c4db874ed --- /dev/null +++ b/src/shared/roomslist/templates/room-item.js @@ -0,0 +1,116 @@ +/** + * @typedef {import('plugins/roomslist/view').RoomsList} RoomsList + * @typedef {import('plugins/bookmark-views/components/bookmarks-pin-list').PinnedBookmarksView} PinnedBookmarksView + * @typedef {import('@converse/headless').MUC} MUC + */ +import { html } from "lit"; +import { api, u } from "@converse/headless"; +import 'plugins/muc-views/modals/add-muc.js'; +import 'plugins/muc-views/modals/muc-list.js'; +import { __ } from 'i18n'; +import { getUnreadMsgsDisplay } from "shared/chat/utils.js"; + +const { isUniView } = u; + +/** @param {MUC} room */ +function isCurrentlyOpen (room) { + return isUniView() && !room.get('hidden'); +} + +/** @param {MUC} room */ +function tplUnreadIndicator (room) { + return html`${ getUnreadMsgsDisplay(room) }`; +} + +function tplActivityIndicator () { + return html``; +} + +/** + * @param {RoomsList|PinnedBookmarksView} el + * @param {MUC} room + */ +export function tplRoomItem (el, room) { + const i18n_leave_room = __('Leave this groupchat'); + const has_unread_msgs = room.get('num_unread_general') || room.get('has_activity'); + + const buttons = [ + tplRoomMenuItem({ + room, + alt_text: i18n_leave_room, + text: __('Leave'), + icon_class: 'fa-sign-out-alt', + btn_class: 'close-room', + handler: (ev) => el.closeRoom(ev) + }), + ]; + + if (api.settings.get('allow_bookmarks')) { + if (!room.bookmark?.get('pinned')) { + buttons.push(tplRoomMenuItem({ + room, + alt_text: __('Pin this groupchat to the top of the list'), + text: __('Pin'), + icon_class: 'fa-bookmark', + btn_class: 'pin-room', + handler: (ev) => el.pinRoom(ev) + })) + } else { + buttons.push(tplRoomMenuItem({ + room, + alt_text: __('Unpin this groupchat from the top of the list'), + text: __('Unpin'), + icon_class: 'fa-bookmark-empty', + btn_class: 'unpin-room', + handler: (ev) => el.unpinRoom(ev) + })) + } + } + + return html` +
  • + + el.openRoom(ev)}> + + ${ room.get('num_unread') ? + tplUnreadIndicator(room) : + (room.get('has_activity') ? tplActivityIndicator() : '') } + ${room.getDisplayName()} + + + +
    • `; +} + +/** + * @param {Object} config + * @param {MUC} config.room + * @param {string} config.alt_text + * @param {string} config.text + * @param {function} config.handler + * @param {string} config.icon_class + * @param {string} config.btn_class + * @returns + */ +function tplRoomMenuItem (config) { + const { room, alt_text, text, handler, icon_class, btn_class } = config; + return html` + + ${text} + `; +} diff --git a/src/types/plugins/bookmark-views/components/bookmarks-pin-list.d.ts b/src/types/plugins/bookmark-views/components/bookmarks-pin-list.d.ts new file mode 100644 index 0000000000..a4e7c4b1cf --- /dev/null +++ b/src/types/plugins/bookmark-views/components/bookmarks-pin-list.d.ts @@ -0,0 +1,5 @@ +export class PinnedBookmarksView extends RoomsList { + model: any; +} +import { RoomsList } from 'plugins/roomslist/view'; +//# sourceMappingURL=bookmarks-pin-list.d.ts.map \ No newline at end of file diff --git a/src/types/plugins/bookmark-views/components/templates/pin-list.d.ts b/src/types/plugins/bookmark-views/components/templates/pin-list.d.ts new file mode 100644 index 0000000000..8a29663ba1 --- /dev/null +++ b/src/types/plugins/bookmark-views/components/templates/pin-list.d.ts @@ -0,0 +1,3 @@ +declare function _default(el: import("plugins/bookmark-views/components/bookmarks-pin-list").PinnedBookmarksView): import("lit-html").TemplateResult<1>; +export default _default; +//# sourceMappingURL=pin-list.d.ts.map \ No newline at end of file diff --git a/src/types/plugins/controlbox/model.d.ts b/src/types/plugins/controlbox/model.d.ts index 9dafc68b5d..50551599eb 100644 --- a/src/types/plugins/controlbox/model.d.ts +++ b/src/types/plugins/controlbox/model.d.ts @@ -9,7 +9,6 @@ export default ControlBox; declare class ControlBox extends Model { constructor(attributes?: Partial, options?: import("@converse/skeletor").ModelOptions); defaults(): { - bookmarked: boolean; box_id: string; chat_state: any; closed: boolean; diff --git a/src/types/plugins/roomslist/view.d.ts b/src/types/plugins/roomslist/view.d.ts index dfd7c82bf6..973ddac74b 100644 --- a/src/types/plugins/roomslist/view.d.ts +++ b/src/types/plugins/roomslist/view.d.ts @@ -1,5 +1,6 @@ export class RoomsList extends CustomElement { model: RoomsListModel; + addEventListeners(): void; render(): import("lit-html").TemplateResult<1>; /** @param {import('@converse/headless').Model} model */ renderIfChatRoom(model: import("@converse/headless").Model): void; @@ -11,6 +12,10 @@ export class RoomsList extends CustomElement { openRoom(ev: Event): Promise; /** @param {Event} ev */ closeRoom(ev: Event): Promise; + /** @param {Event} ev */ + pinRoom(ev: Event): void; + /** @param {Event} ev */ + unpinRoom(ev: Event): void; /** @param {Event} [ev] */ toggleRoomsList(ev?: Event): void; /** diff --git a/src/types/shared/roomslist/templates/room-item.d.ts b/src/types/shared/roomslist/templates/room-item.d.ts new file mode 100644 index 0000000000..cacb282c56 --- /dev/null +++ b/src/types/shared/roomslist/templates/room-item.d.ts @@ -0,0 +1,9 @@ +/** + * @param {RoomsList|PinnedBookmarksView} el + * @param {MUC} room + */ +export function tplRoomItem(el: RoomsList | PinnedBookmarksView, room: MUC): import("lit-html").TemplateResult<1>; +export type RoomsList = import("plugins/roomslist/view").RoomsList; +export type PinnedBookmarksView = import("plugins/bookmark-views/components/bookmarks-pin-list").PinnedBookmarksView; +export type MUC = import("@converse/headless").MUC; +//# sourceMappingURL=room-item.d.ts.map \ No newline at end of file From f24bb9a6affa15d6439f6cb02823ce67e023d16d Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 18 Jun 2026 11:12:53 +0200 Subject: [PATCH 2/4] fix(bookmarks): robust pin linkage and single source of truth for pinned state Review fixes on top of the XEP-0469 pinning implementation: - Establish the bookmark<->MUC link whenever either side appears: on bookmark-add, on chatbox-add, and via a one-off sweep after bookmarks are fetched. - Derive `pinned` from the bookmark's `` extension (single source of truth) instead of storing a separate cached boolean. - Simplify pinBookmark/unpinBookmark to only mutate extensions and return the promise; drop the unreachable try/catch rollback around the async api.bookmarks.set call. - Make `setBookmark` idempotent and clear the link when a bookmark is removed. - Move `ModelWithBookmark` onto MUC only. - Remove the now-vestigial setBookmarkState mixin. - Restrict the pinned list to open group chats (type + !closed) and drop dead modal imports from the shared room-item template. --- src/headless/plugins/bookmarks/collection.js | 105 ++++++++++-------- src/headless/plugins/bookmarks/model.js | 15 ++- src/headless/plugins/bookmarks/plugin.js | 1 + .../plugins/bookmarks/tests/bookmarks.js | 44 ++++++++ src/headless/plugins/chat/model.js | 5 +- src/headless/shared/chatbox.js | 3 +- src/headless/shared/model-with-bookmark.js | 12 +- .../types/plugins/bookmarks/collection.d.ts | 19 +++- .../types/plugins/bookmarks/model.d.ts | 1 + src/headless/types/plugins/chat/model.d.ts | 72 ------------ src/headless/types/plugins/muc/muc.d.ts | 2 +- src/headless/types/shared/chatbox.d.ts | 72 ------------ .../types/shared/model-with-bookmark.d.ts | 8 +- .../components/bookmarks-pin-list.js | 4 +- src/plugins/bookmark-views/index.js | 7 +- src/plugins/bookmark-views/mixins.js | 16 --- .../tests/bookmarks-pin-list.js | 32 ++++++ src/shared/roomslist/templates/room-item.js | 2 - src/types/plugins/bookmark-views/mixins.d.ts | 5 - 19 files changed, 187 insertions(+), 238 deletions(-) diff --git a/src/headless/plugins/bookmarks/collection.js b/src/headless/plugins/bookmarks/collection.js index 99da3dd36c..82e3e6b21c 100644 --- a/src/headless/plugins/bookmarks/collection.js +++ b/src/headless/plugins/bookmarks/collection.js @@ -32,26 +32,38 @@ class Bookmarks extends Collection { } async initialize() { + const { chatboxes } = _converse.state; + this.on('add', (bm) => this.openBookmarkedRoom(bm) - .then((bm) => this.markRoomAsBookmarked(bm)) - .catch((e) => log.fatal(e)) + .then((bm) => this.linkRoom(bm.get('jid'))) + .catch((e) => log.fatal(e)), ); this.on('change:autojoin', this.onAutoJoinChanged, this); this.on( 'remove', - /** @param { Bookmark } bookmark }*/ (bookmark) => { + /** @param {Bookmark} bookmark */ (bookmark) => { + chatboxes.get(bookmark.get('jid'))?.setBookmark?.(null); this.sendRemoveBookmarkStanza(bookmark); this.leaveRoom(bookmark); - } + }, ); + // A room may be opened *after* its bookmark already exists (e.g. a + // non-autojoin bookmark that the user opens manually), so we link on + // chatbox-add as well as on bookmark-add. + this.listenTo(chatboxes, 'add', /** @param {MUC} cb */ (cb) => this.linkRoom(cb.get('jid'))); + const { storage_key, fetched_flag_key } = getStorageKeys(); this.fetched_flag = fetched_flag_key; initStorage(this, storage_key); await this.fetchBookmarks(); + // Reconcile any rooms that were already open before the bookmarks + // finished loading. + chatboxes.forEach(/** @param {MUC} cb */ (cb) => this.linkRoom(cb.get('jid'))); + /** * Triggered once the {@link Bookmarks} collection * has been created and cached bookmarks have been fetched. @@ -82,7 +94,6 @@ class Bookmarks extends Collection { const groupchat = await api.rooms.create(bookmark.get('jid'), { nick: bookmark.get('nick'), password: bookmark.get('password'), - pinned: bookmark.get('pinned'), }); groupchat.maybeShow(); } @@ -169,18 +180,27 @@ class Bookmarks extends Collection { if (node === Strophe.NS.BOOKMARKS2) { if (!bookmark) throw new Error('getPublishedItems: missing bookmark'); - const extensions = bookmark.get('extensions') ?? []; + // Parse each extension defensively + const extensions = (bookmark.get('extensions') ?? []) + .map( + /** @param {string} e */ (e) => { + try { + return Stanza.fromString(e); + } catch (err) { + log.error(`Ignoring invalid bookmark extension: ${e}`); + log.error(err); + return null; + } + }, + ) + .filter(Boolean); return stx` ${bookmark.get('nick') ? stx`${bookmark.get('nick')}` : ''} ${bookmark.get('password') ? stx`${bookmark.get('password')}` : ''} - ${ - extensions.length - ? stx`${extensions.map((e) => Stanza.fromString(e))}` - : '' - } + ${extensions.length ? stx`${extensions}` : ''} `; } else { @@ -192,7 +212,7 @@ class Bookmarks extends Collection { jid="${model.get('jid')}"> ${model.get('nick') ? stx`${model.get('nick')}` : ''} ${model.get('password') ? stx`${model.get('password')}` : ''} - ` + `, )} `; @@ -246,12 +266,14 @@ class Bookmarks extends Collection { } /** - * @param {Bookmark} bookmark + * Associate an open room with its bookmark, if both exist. Safe to call + * repeatedly (see {@link ModelWithBookmark#setBookmark}) and for any chatbox + * type — non-MUC boxes simply have no `setBookmark` method. + * @param {string} jid */ - markRoomAsBookmarked(bookmark) { + linkRoom(jid) { const { chatboxes } = _converse.state; - const groupchat = chatboxes.get(bookmark.get('jid')); - groupchat?.setBookmark(bookmark); + chatboxes.get(jid)?.setBookmark?.(this.get(jid) ?? null); } /** @@ -283,7 +305,7 @@ class Bookmarks extends Collection { (attrs) => { const bookmark = this.get(attrs.jid); bookmark ? bookmark.save(attrs) : this.create(attrs); - } + }, ); } @@ -310,7 +332,7 @@ class Bookmarks extends Collection { api.alert('error', __('Timeout Error'), [ __( 'The server did not return your bookmarks within the allowed time. ' + - 'You can reload the page to request them again.' + 'You can reload the page to request them again.', ), ]); deferred?.reject(new Error('Could not fetch bookmarks')); @@ -337,45 +359,32 @@ class Bookmarks extends Collection { } /** - * + * Pin a bookmark to the top of the lists (XEP-0469) by adding a `` + * element to its extensions. The `pinned` attribute is derived from the + * extensions by {@link Bookmark}, so we only need to update the latter. * @param {Bookmark} bookmark + * @returns {Promise} */ pinBookmark(bookmark) { - const extensions = [...bookmark.get('extensions'), ``]; - - bookmark.set('pinned', true); - - try { - api.bookmarks.set({ - jid: bookmark.get('jid'), - extensions, - }); - } catch (error) { - bookmark.set('pinned', false); - log.error('Error while trying to pin bookmark'); - log.error(error); - } + if (bookmark.get('pinned')) return Promise.resolve(); + const extensions = [ + ...(bookmark.get('extensions') ?? []), + ``, + ]; + return api.bookmarks.set({ jid: bookmark.get('jid'), extensions }); } /** - * + * Unpin a bookmark (XEP-0469) by removing its `` extension. * @param {Bookmark} bookmark + * @returns {Promise} */ unpinBookmark(bookmark) { - const extensions = bookmark.get('extensions').filter(/** @param {String} e */ e => !(e.includes(' !(e.includes(' e.includes('` element + // in `extensions`, which is the single source of truth. Keep it in sync + // whenever the extensions change. + this.on('change:extensions', () => this.updatePinnedState()); + this.updatePinnedState(); } get idAttribute() { return 'jid'; } + updatePinnedState() { + const ns = Strophe.NS.BOOKMARKS_PINNING; + const pinned = + this.get('extensions')?.some( + /** @param {string} e */ (e) => e.includes(' { const { state } = _converse; if (state.bookmarks) { + state.bookmarks.stopListening(); state.bookmarks.clearStore({ silent: true }); const { fetched_flag_key } = getStorageKeys(); _converse.state.session.set(fetched_flag_key, undefined); diff --git a/src/headless/plugins/bookmarks/tests/bookmarks.js b/src/headless/plugins/bookmarks/tests/bookmarks.js index 8395332fb4..8eaf082c06 100644 --- a/src/headless/plugins/bookmarks/tests/bookmarks.js +++ b/src/headless/plugins/bookmarks/tests/bookmarks.js @@ -712,4 +712,48 @@ describe('A bookmark', function () { `); }), ); + + it( + 'drops malformed extensions instead of breaking the publish', + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + + const muc_jid = 'theplay@conference.shakespeare.lit'; + const { state } = _converse; + + // A bookmark carrying a malformed extension alongside a valid one. + state.bookmarks.create({ + jid: muc_jid, + autojoin: true, + name: 'The Play', + nick: 'romeo', + extensions: ['`], + }); + + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + await u.waitUntil(() => state.chatboxes.length === 1); + + const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; + const bookmark = state.bookmarks.findWhere({ jid: muc_jid }); + + // Serializing/publishing must not throw: the malformed extension is + // dropped and the valid extension is preserved. (We don't + // await the publish itself, since the mock server never answers the + // IQ; we only care that the stanza was built and sent.) + state.bookmarks.sendBookmarkStanza(bookmark).catch(() => {}); + + const sent_stanza = await u.waitUntil(() => + IQ_stanzas.filter( + (s) => sizzle(`publish[node="${Strophe.NS.BOOKMARKS2}"] conference[name="The Play"]`, s).length + ).pop() + ); + + const extensions_els = sizzle('extensions', sent_stanza); + expect(extensions_els.length).toBe(1); + expect(extensions_els[0].children.length).toBe(1); + expect(extensions_els[0].children[0].tagName).toBe('pinned'); + expect(extensions_els[0].children[0].namespaceURI).toBe(Strophe.NS.BOOKMARKS_PINNING); + }), + ); }); diff --git a/src/headless/plugins/chat/model.js b/src/headless/plugins/chat/model.js index bcaf58ffc8..b2374f807e 100644 --- a/src/headless/plugins/chat/model.js +++ b/src/headless/plugins/chat/model.js @@ -6,7 +6,6 @@ import converse from '../../shared/api/public.js'; import log from '@converse/log'; import { isUniView } from '../../utils/session.js'; import { sendChatState, sendMarker } from '../../shared/actions.js'; -import ModelWithBookmark from '../../shared/model-with-bookmark.js'; import ModelWithMessages from '../../shared/model-with-messages.js'; import ModelWithVCard from '../../shared/model-with-vcard.js'; import ModelWithContact from '../../shared/model-with-contact.js'; @@ -18,9 +17,7 @@ const { Strophe, u } = converse.env; /** * Represents a one-on-one chat conversation. */ -class ChatBox extends ModelWithBookmark( - ModelWithVCard(ModelWithMessages(ModelWithContact(ColorAwareModel(ChatBoxBase)))) -) { +class ChatBox extends ModelWithVCard(ModelWithMessages(ModelWithContact(ColorAwareModel(ChatBoxBase)))) { /** * @typedef {import('./message.js').default} Message * @typedef {import('../muc/muc.js').default} MUC diff --git a/src/headless/shared/chatbox.js b/src/headless/shared/chatbox.js index a87f5bfe82..a4f4a4a612 100644 --- a/src/headless/shared/chatbox.js +++ b/src/headless/shared/chatbox.js @@ -5,14 +5,13 @@ import _converse from './_converse.js'; import converse from './api/public.js'; import log from '@converse/log'; import ModelWithMessages from './model-with-messages.js'; -import ModelWithBookmark from './model-with-bookmark.js'; const { u } = converse.env; /** * Base class for all chat boxes. Provides common methods. */ -export default class ChatBoxBase extends ModelWithBookmark(ModelWithMessages(Model)) { +export default class ChatBoxBase extends ModelWithMessages(Model) { async initialize() { await super.initialize(); const jid = this.get('jid'); diff --git a/src/headless/shared/model-with-bookmark.js b/src/headless/shared/model-with-bookmark.js index 0c226e07a4..b4c7d8e845 100644 --- a/src/headless/shared/model-with-bookmark.js +++ b/src/headless/shared/model-with-bookmark.js @@ -10,9 +10,19 @@ export default function ModelWithBookmark(BaseModel) { this.bookmark = null; } + /** + * Associate this model with a bookmark (or clear it by passing `null`). + * Idempotent: re-binding the same bookmark is a no-op, and rebinding a + * different one detaches the previous listener first. + * @param {import('@converse/skeletor').Model|null} bookmark + */ setBookmark(bookmark) { + if (this.bookmark === bookmark) return; + if (this.bookmark) this.stopListening(this.bookmark); this.bookmark = bookmark; - this.listenTo(this.bookmark, 'change', () => this.trigger('bookmark:change', bookmark)); + if (bookmark) { + this.listenTo(bookmark, 'change', () => this.trigger('bookmark:change', bookmark)); + } this.trigger('bookmark:change', bookmark); } }; diff --git a/src/headless/types/plugins/bookmarks/collection.d.ts b/src/headless/types/plugins/bookmarks/collection.d.ts index 4ffdca2a06..a5fbdb82e8 100644 --- a/src/headless/types/plugins/bookmarks/collection.d.ts +++ b/src/headless/types/plugins/bookmarks/collection.d.ts @@ -52,9 +52,12 @@ declare class Bookmarks extends Collection { */ fetchBookmarksFromServer(deferred: Promise): Promise; /** - * @param {Bookmark} bookmark + * Associate an open room with its bookmark, if both exist. Safe to call + * repeatedly (see {@link ModelWithBookmark#setBookmark}) and for any chatbox + * type — non-MUC boxes simply have no `setBookmark` method. + * @param {string} jid */ - markRoomAsBookmarked(bookmark: Bookmark): void; + linkRoom(jid: string): void; /** * @param {Bookmark} bookmark */ @@ -79,15 +82,19 @@ declare class Bookmarks extends Collection { onBookmarksReceivedError(deferred: any, iq: Element): Promise; getUnopenedBookmarks(): Promise; /** - * + * Pin a bookmark to the top of the lists (XEP-0469) by adding a `` + * element to its extensions. The `pinned` attribute is derived from the + * extensions by {@link Bookmark}, so we only need to update the latter. * @param {Bookmark} bookmark + * @returns {Promise} */ - pinBookmark(bookmark: Bookmark): void; + pinBookmark(bookmark: Bookmark): Promise; /** - * + * Unpin a bookmark (XEP-0469) by removing its `` extension. * @param {Bookmark} bookmark + * @returns {Promise} */ - unpinBookmark(bookmark: Bookmark): void; + unpinBookmark(bookmark: Bookmark): Promise; } import Bookmark from './model.js'; import { Collection } from '@converse/skeletor'; diff --git a/src/headless/types/plugins/bookmarks/model.d.ts b/src/headless/types/plugins/bookmarks/model.d.ts index 4b62200ecb..55d789b8ba 100644 --- a/src/headless/types/plugins/bookmarks/model.d.ts +++ b/src/headless/types/plugins/bookmarks/model.d.ts @@ -2,6 +2,7 @@ export default Bookmark; declare class Bookmark extends Model { constructor(attributes?: Partial, options?: import("@converse/skeletor").ModelOptions); initialize(): void; + updatePinnedState(): void; getDisplayName(): any; } import { Model } from '@converse/skeletor'; diff --git a/src/headless/types/plugins/chat/model.d.ts b/src/headless/types/plugins/chat/model.d.ts index 2d9ee492f3..175fc287e7 100644 --- a/src/headless/types/plugins/chat/model.d.ts +++ b/src/headless/types/plugins/chat/model.d.ts @@ -1,77 +1,5 @@ export default ChatBox; declare const ChatBox_base: { - new (...args: any[]): { - [x: string]: any; - initialize(): void; - bookmark: any; - setBookmark(bookmark: any): void; - _browserStorage?: import("@converse/skeletor").BrowserStorage; - _changing: boolean; - _pending: boolean | import("@converse/skeletor").ModelOptions; - _previousAttributes?: import("@converse/skeletor").ModelAttributes; - _url: string; - _urlRoot: string; - attributes: import("@converse/skeletor").ModelAttributes; - changed: Partial; - cid: string; - collection?: import("@converse/skeletor").Collection; - id: string | number; - validationError: string | number | null; - browserStorage: import("@converse/skeletor").BrowserStorage; - readonly idAttribute: string; - readonly cidPrefix: string; - preinitialize(...args: any[]): void; - validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; - defaults(): Partial; - toJSON(): import("@converse/skeletor").ModelAttributes; - sync(method: import("@converse/skeletor").SyncOperation, model: import("@converse/skeletor").Model, options: import("@converse/skeletor").Options): any; - get(attr: K): import("@converse/skeletor").ModelAttributes[K]; - keys(): string[]; - values(): any[]; - pairs(): [string | number, any][]; - entries(): [string | number, any][]; - invert(): Record; - pick(...args: K[]): Pick; - omit(...args: K[]): Omit; - isEmpty(): boolean; - has(attr: string | number): boolean; - matches(attrs: Partial): boolean; - set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; - unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; - clear(options?: import("@converse/skeletor").ModelOptions): any; - hasChanged(attr?: string | number): boolean; - changedAttributes(diff?: Partial): false | Partial; - previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; - previousAttributes(): import("@converse/skeletor").ModelAttributes; - fetch(options?: import("@converse/skeletor").Options): any; - save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; - destroy(options?: import("@converse/skeletor").ModelOptions): any; - urlRoot: string; - url: string; - parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; - isNew(): boolean; - isValid(options?: import("@converse/skeletor").ModelOptions): boolean; - _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; - _events?: import("@converse/skeletor").EventHandlersMap; - _listeners?: import("@converse/skeletor").EventListenerMap; - _listeningTo?: import("@converse/skeletor").EventListenerMap; - _listenId?: string; - on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; - listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; - stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; - listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - trigger(name: string, ...args: any[]): any; - constructor: Function; - toString(): string; - toLocaleString(): string; - valueOf(): Object; - hasOwnProperty(v: PropertyKey): boolean; - isPrototypeOf(v: Object): boolean; - propertyIsEnumerable(v: PropertyKey): boolean; - }; -} & { new (...args: any[]): { [x: string]: any; _vcard: import("../vcard/vcard.js").default; diff --git a/src/headless/types/plugins/muc/muc.d.ts b/src/headless/types/plugins/muc/muc.d.ts index 7c054e60c1..430f4b66e9 100644 --- a/src/headless/types/plugins/muc/muc.d.ts +++ b/src/headless/types/plugins/muc/muc.d.ts @@ -4,7 +4,7 @@ declare const MUC_base: { [x: string]: any; initialize(): void; bookmark: any; - setBookmark(bookmark: any): void; + setBookmark(bookmark: import("@converse/skeletor").Model | null): void; _browserStorage?: import("@converse/skeletor").BrowserStorage; _changing: boolean; _pending: boolean | import("@converse/skeletor").ModelOptions; diff --git a/src/headless/types/shared/chatbox.d.ts b/src/headless/types/shared/chatbox.d.ts index 2baa9307fa..3ffdce8f92 100644 --- a/src/headless/types/shared/chatbox.d.ts +++ b/src/headless/types/shared/chatbox.d.ts @@ -1,76 +1,4 @@ declare const ChatBoxBase_base: { - new (...args: any[]): { - [x: string]: any; - initialize(): void; - bookmark: any; - setBookmark(bookmark: any): void; - _browserStorage?: import("@converse/skeletor").BrowserStorage; - _changing: boolean; - _pending: boolean | import("@converse/skeletor").ModelOptions; - _previousAttributes?: import("@converse/skeletor").ModelAttributes; - _url: string; - _urlRoot: string; - attributes: import("@converse/skeletor").ModelAttributes; - changed: Partial; - cid: string; - collection?: import("@converse/skeletor").Collection; - id: string | number; - validationError: string | number | null; - browserStorage: import("@converse/skeletor").BrowserStorage; - readonly idAttribute: string; - readonly cidPrefix: string; - preinitialize(...args: any[]): void; - validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): string | number | null | void; - defaults(): Partial; - toJSON(): import("@converse/skeletor").ModelAttributes; - sync(method: import("@converse/skeletor").SyncOperation, model: Model, options: import("@converse/skeletor").Options): any; - get(attr: K): import("@converse/skeletor").ModelAttributes[K]; - keys(): string[]; - values(): any[]; - pairs(): [string | number, any][]; - entries(): [string | number, any][]; - invert(): Record; - pick(...args: K[]): Pick; - omit(...args: K[]): Omit; - isEmpty(): boolean; - has(attr: string | number): boolean; - matches(attrs: Partial): boolean; - set(key: string | import("@converse/skeletor").ObjectWithId | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; - unset(attr: string | number, options?: import("@converse/skeletor").ModelOptions): any; - clear(options?: import("@converse/skeletor").ModelOptions): any; - hasChanged(attr?: string | number): boolean; - changedAttributes(diff?: Partial): false | Partial; - previous(attr: K): import("@converse/skeletor").ModelAttributes[K]; - previousAttributes(): import("@converse/skeletor").ModelAttributes; - fetch(options?: import("@converse/skeletor").Options): any; - save(key?: string | Partial, val?: any, options?: import("@converse/skeletor").ModelOptions): any; - destroy(options?: import("@converse/skeletor").ModelOptions): any; - urlRoot: string; - url: string; - parse(resp: any, options?: import("@converse/skeletor").ModelOptions): void | Partial; - isNew(): boolean; - isValid(options?: import("@converse/skeletor").ModelOptions): boolean; - _validate(attrs: import("@converse/skeletor").ObjectWithId | Partial, options?: import("@converse/skeletor").ModelOptions): boolean; - _events?: import("@converse/skeletor").EventHandlersMap; - _listeners?: import("@converse/skeletor").EventListenerMap; - _listeningTo?: import("@converse/skeletor").EventListenerMap; - _listenId?: string; - on(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; - listenTo(obj: import("@converse/skeletor").ObjectListenedTo, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - off(name?: string | import("@converse/skeletor").EventCallbackMap | null, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext | null, context?: import("@converse/skeletor").EventContext): any; - stopListening(obj?: any, name?: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - once(name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback | import("@converse/skeletor").EventContext, context?: import("@converse/skeletor").EventContext): any; - listenToOnce(obj: any, name: string | import("@converse/skeletor").EventCallbackMap, callback?: import("@converse/skeletor").EventCallback): any; - trigger(name: string, ...args: any[]): any; - constructor: Function; - toString(): string; - toLocaleString(): string; - valueOf(): Object; - hasOwnProperty(v: PropertyKey): boolean; - isPrototypeOf(v: Object): boolean; - propertyIsEnumerable(v: PropertyKey): boolean; - }; -} & { new (...args: any[]): { [x: string]: any; disable_mam: boolean; diff --git a/src/headless/types/shared/model-with-bookmark.d.ts b/src/headless/types/shared/model-with-bookmark.d.ts index 3b9baf723e..cf4fe27f94 100644 --- a/src/headless/types/shared/model-with-bookmark.d.ts +++ b/src/headless/types/shared/model-with-bookmark.d.ts @@ -7,7 +7,13 @@ export default function ModelWithBookmark m.bookmark?.get('pinned')); + const rooms = chatboxes.filter( + (m) => m.get('type') === constants.CHATROOMS_TYPE && !m.get('closed') && m.bookmark?.get('pinned') + ); rooms.sort((a, b) => (a.getDisplayName().toLowerCase() <= b.getDisplayName().toLowerCase() ? -1 : 1)); return rooms; } diff --git a/src/plugins/bookmark-views/index.js b/src/plugins/bookmark-views/index.js index 82f8efcb9d..93202073d3 100644 --- a/src/plugins/bookmark-views/index.js +++ b/src/plugins/bookmark-views/index.js @@ -22,7 +22,7 @@ converse.plugins.add('converse-bookmark-views', { * an error will be raised if the plugin is not found. By default it's * false, which means these plugins are only loaded opportunistically. */ - dependencies: ['converse-chatboxes', 'converse-muc', 'converse-muc-views'], + dependencies: ['converse-chatboxes', 'converse-muc', 'converse-muc-views', 'converse-roomslist'], initialize() { // Configuration values for this plugin @@ -43,10 +43,5 @@ converse.plugins.add('converse-bookmark-views', { Object.assign(_converse, exports); // DEPRECATED Object.assign(_converse.exports, exports); Object.assign(_converse.exports.ChatRoomView.prototype, BookmarkableChatRoomView); - - api.listen.on( - 'chatRoomViewInitialized', - /** @param {BookmarkableChatRoomView} view */ (view) => view.setBookmarkState() - ); }, }); diff --git a/src/plugins/bookmark-views/mixins.js b/src/plugins/bookmark-views/mixins.js index 4fe5101b65..92e898257c 100644 --- a/src/plugins/bookmark-views/mixins.js +++ b/src/plugins/bookmark-views/mixins.js @@ -3,22 +3,6 @@ import { _converse, api, converse } from '@converse/headless'; const { u } = converse.env; export const BookmarkableChatRoomView = { - /** - * Set whether the groupchat is bookmarked or not. - * @private - */ - setBookmarkState() { - const { bookmarks } = _converse.state; - if (bookmarks) { - const models = bookmarks.where({ jid: this.model.get('jid') }); - if (!models.length) { - this.model.save('bookmarked', false); - } else { - this.model.save('bookmarked', true); - } - } - }, - renderBookmarkForm() { if (!this.bookmark_form) { this.bookmark_form = new _converse.state.MUCBookmarkForm({ diff --git a/src/plugins/bookmark-views/tests/bookmarks-pin-list.js b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js index cec499e123..aa9ff7eeb6 100644 --- a/src/plugins/bookmark-views/tests/bookmarks-pin-list.js +++ b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js @@ -55,4 +55,36 @@ describe("The bookmarks pin list", function () { expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(2); expect(main_list.querySelectorAll(".open-room").length).toBe(0); })); + + it("shows a room opened manually after it was already a pinned bookmark", + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + const { api } = _converse; + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + await mock.openControlBox(_converse); + + const bookmarks_pin_list = document.querySelector('converse-pinned-bookmarks'); + const main_list = document.querySelector('converse-rooms-list'); + + const muc_jid = 'lounge@montague.lit'; + // A pinned bookmark that is NOT auto-joined: no room is open yet, + // so nothing should appear in either list. + await api.bookmarks.set({ + jid: muc_jid, + name: 'Lounge', + autojoin: false, + nick: 'romeo', + extensions: [``], + }); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(0); + + // Open the room manually, *after* the bookmark already exists. + api.rooms.open(muc_jid, { nick: 'romeo' }); + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + + // It must show up in the pinned list, not the regular rooms list. + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length === 1); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(1); + expect(main_list.querySelectorAll(".open-room").length).toBe(0); + })); }); diff --git a/src/shared/roomslist/templates/room-item.js b/src/shared/roomslist/templates/room-item.js index 8c4db874ed..aab1716ef2 100644 --- a/src/shared/roomslist/templates/room-item.js +++ b/src/shared/roomslist/templates/room-item.js @@ -5,8 +5,6 @@ */ import { html } from "lit"; import { api, u } from "@converse/headless"; -import 'plugins/muc-views/modals/add-muc.js'; -import 'plugins/muc-views/modals/muc-list.js'; import { __ } from 'i18n'; import { getUnreadMsgsDisplay } from "shared/chat/utils.js"; diff --git a/src/types/plugins/bookmark-views/mixins.d.ts b/src/types/plugins/bookmark-views/mixins.d.ts index 6b5ebfd04a..0520f7c2c1 100644 --- a/src/types/plugins/bookmark-views/mixins.d.ts +++ b/src/types/plugins/bookmark-views/mixins.d.ts @@ -1,9 +1,4 @@ export namespace BookmarkableChatRoomView { - /** - * Set whether the groupchat is bookmarked or not. - * @private - */ - function setBookmarkState(): void; function renderBookmarkForm(): void; function showBookmarkModal(ev: Event): void; } From ef6803cf73a38e79a0904c3630b3991a6cae1888 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Sun, 21 Jun 2026 20:55:24 +0200 Subject: [PATCH 3/4] fix: when pinning an unbookmarked room, bookmark it first --- src/headless/plugins/bookmarks/collection.js | 36 ++++++++++++++++--- .../types/plugins/bookmarks/collection.d.ts | 9 +++++ .../tests/bookmarks-pin-list.js | 34 ++++++++++++++++++ src/plugins/roomslist/view.js | 6 +--- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/headless/plugins/bookmarks/collection.js b/src/headless/plugins/bookmarks/collection.js index 82e3e6b21c..ca0892b325 100644 --- a/src/headless/plugins/bookmarks/collection.js +++ b/src/headless/plugins/bookmarks/collection.js @@ -18,6 +18,14 @@ import { getStorageKeys } from './utils.js'; const { Strophe, stx } = converse.env; +/** + * The `` extension element (XEP-0469), serialized as a string. + * Wrapped in a function so the namespace (registered in the plugin) is only + * read at call time, not at module load. + * @returns {string} + */ +const getPinnedExtension = () => ``; + /** * @extends {Collection} */ @@ -367,13 +375,33 @@ class Bookmarks extends Collection { */ pinBookmark(bookmark) { if (bookmark.get('pinned')) return Promise.resolve(); - const extensions = [ - ...(bookmark.get('extensions') ?? []), - ``, - ]; + const extensions = [...(bookmark.get('extensions') ?? []), getPinnedExtension()]; return api.bookmarks.set({ jid: bookmark.get('jid'), extensions }); } + /** + * Pin a room to the top of the lists (XEP-0469). Pinning is an extension on + * a bookmark, so if the room isn't bookmarked yet we bookmark it first + * (with autojoin enabled, so the pin survives a reload) and include the + * `` extension in the same publish. + * @param {string} jid + * @returns {Promise} + */ + pinRoom(jid) { + const bookmark = this.get(jid); + if (bookmark) return this.pinBookmark(bookmark); + + const room = _converse.state.chatboxes.get(jid); + return api.bookmarks.set({ + jid, + name: room?.get('name'), + nick: room?.get('nick'), + password: room?.get('password'), + autojoin: true, + extensions: [getPinnedExtension()], + }); + } + /** * Unpin a bookmark (XEP-0469) by removing its `` extension. * @param {Bookmark} bookmark diff --git a/src/headless/types/plugins/bookmarks/collection.d.ts b/src/headless/types/plugins/bookmarks/collection.d.ts index a5fbdb82e8..dbe55b2349 100644 --- a/src/headless/types/plugins/bookmarks/collection.d.ts +++ b/src/headless/types/plugins/bookmarks/collection.d.ts @@ -89,6 +89,15 @@ declare class Bookmarks extends Collection { * @returns {Promise} */ pinBookmark(bookmark: Bookmark): Promise; + /** + * Pin a room to the top of the lists (XEP-0469). Pinning is an extension on + * a bookmark, so if the room isn't bookmarked yet we bookmark it first + * (with autojoin enabled, so the pin survives a reload) and include the + * `` extension in the same publish. + * @param {string} jid + * @returns {Promise} + */ + pinRoom(jid: string): Promise; /** * Unpin a bookmark (XEP-0469) by removing its `` extension. * @param {Bookmark} bookmark diff --git a/src/plugins/bookmark-views/tests/bookmarks-pin-list.js b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js index aa9ff7eeb6..e73f9f62a6 100644 --- a/src/plugins/bookmark-views/tests/bookmarks-pin-list.js +++ b/src/plugins/bookmark-views/tests/bookmarks-pin-list.js @@ -87,4 +87,38 @@ describe("The bookmarks pin list", function () { expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(1); expect(main_list.querySelectorAll(".open-room").length).toBe(0); })); + + it("bookmarks and pins a room that was not bookmarked yet", + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + const { api, state } = _converse; + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + await mock.openControlBox(_converse); + + const bookmarks_pin_list = document.querySelector('converse-pinned-bookmarks'); + const main_list = document.querySelector('converse-rooms-list'); + + const muc_jid = 'lounge@montague.lit'; + // Open a room *without* bookmarking it. + api.rooms.open(muc_jid, { nick: 'romeo' }); + await mock.waitForMUCDiscoInfo(_converse, muc_jid); + + // It shows in the regular rooms list, not the pinned list, and has + // no bookmark yet. + await u.waitUntil(() => main_list.querySelectorAll(".open-room").length === 1); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(0); + expect(state.bookmarks.get(muc_jid)).toBeUndefined(); + + // Pinning it should bookmark it first, then pin it. + main_list.querySelector('.pin-room').click(); + + await u.waitUntil(() => bookmarks_pin_list.querySelectorAll(".open-room").length === 1); + expect(bookmarks_pin_list.querySelectorAll(".open-room").length).toBe(1); + expect(main_list.querySelectorAll(".open-room").length).toBe(0); + + const bookmark = state.bookmarks.get(muc_jid); + expect(bookmark).toBeTruthy(); + expect(bookmark.get('pinned')).toBe(true); + expect(bookmark.get('autojoin')).toBe(true); + })); }); diff --git a/src/plugins/roomslist/view.js b/src/plugins/roomslist/view.js index 79ee593bce..62aec3ee74 100644 --- a/src/plugins/roomslist/view.js +++ b/src/plugins/roomslist/view.js @@ -92,11 +92,7 @@ export class RoomsList extends CustomElement { ev.preventDefault(); const target = /** @type {HTMLElement} */ (ev.currentTarget); const jid = target.getAttribute('data-room-jid'); - const { bookmarks } = _converse.state; - bookmarks - .where({ jid }) - .forEach(/** @param {import('@converse/headless').Bookmark} b */ (b) => - bookmarks.pinBookmark(b)); + _converse.state.bookmarks.pinRoom(jid); } /** @param {Event} ev */ From 7286a841b28cefd22ebafba9b5233763698590f2 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Sun, 21 Jun 2026 22:13:57 +0200 Subject: [PATCH 4/4] refactor(bookmarks): robust, idempotent XEP-0469 pin/unpin handling Follow-up polish on the pinning code: Pin/unpin now compute the desired extensions declaratively and always (re)publish. This is idempotent and self-healing when local and server state have diverged. --- src/headless/plugins/bookmarks/collection.js | 40 ++++++++--- src/headless/plugins/bookmarks/model.js | 8 +-- .../plugins/bookmarks/tests/bookmarks.js | 72 +++++++++++++++---- src/headless/plugins/bookmarks/utils.js | 21 +++++- .../types/plugins/bookmarks/collection.d.ts | 18 ++++- .../types/plugins/bookmarks/utils.d.ts | 10 +++ src/plugins/roomslist/view.js | 5 +- 7 files changed, 135 insertions(+), 39 deletions(-) diff --git a/src/headless/plugins/bookmarks/collection.js b/src/headless/plugins/bookmarks/collection.js index ca0892b325..8168b089a2 100644 --- a/src/headless/plugins/bookmarks/collection.js +++ b/src/headless/plugins/bookmarks/collection.js @@ -14,7 +14,7 @@ import log from '@converse/log'; import { initStorage } from '../../utils/storage.js'; import { parseStanzaForBookmarks } from './parsers.js'; import '../../plugins/muc/index.js'; -import { getStorageKeys } from './utils.js'; +import { getStorageKeys, isPinnedExtension } from './utils.js'; const { Strophe, stx } = converse.env; @@ -26,6 +26,14 @@ const { Strophe, stx } = converse.env; */ const getPinnedExtension = () => ``; +/** + * Returns a copy of an extensions list with any XEP-0469 `` element(s) + * removed. Other (unknown) extensions are kept, as XEP-0402 requires. + * @param {string[]} [extensions] + * @returns {string[]} + */ +const withoutPinnedExtension = (extensions) => (extensions ?? []).filter((e) => !isPinnedExtension(e)); + /** * @extends {Collection} */ @@ -367,15 +375,18 @@ class Bookmarks extends Collection { } /** - * Pin a bookmark to the top of the lists (XEP-0469) by adding a `` - * element to its extensions. The `pinned` attribute is derived from the - * extensions by {@link Bookmark}, so we only need to update the latter. + * Pin a bookmark to the top of the lists (XEP-0469) by ensuring exactly one + * `` element is present in its extensions. We always (re)publish, + * even when the bookmark already looks pinned locally: the operation is + * idempotent (any existing `` is stripped before re-adding a single + * one, so it can't accumulate duplicates) and self-healing if our local + * state and the server's have diverged. The `pinned` attribute is derived + * from the extensions by {@link Bookmark}. * @param {Bookmark} bookmark * @returns {Promise} */ pinBookmark(bookmark) { - if (bookmark.get('pinned')) return Promise.resolve(); - const extensions = [...(bookmark.get('extensions') ?? []), getPinnedExtension()]; + const extensions = [...withoutPinnedExtension(bookmark.get('extensions')), getPinnedExtension()]; return api.bookmarks.set({ jid: bookmark.get('jid'), extensions }); } @@ -402,16 +413,25 @@ class Bookmarks extends Collection { }); } + /** + * Unpin a room by its JID (XEP-0469). A no-op if the room isn't bookmarked + * (you can only unpin something that was pinned, which requires a bookmark). + * @param {string} jid + * @returns {Promise|void} + */ + unpinRoom(jid) { + const bookmark = this.get(jid); + if (bookmark) return this.unpinBookmark(bookmark); + } + /** * Unpin a bookmark (XEP-0469) by removing its `` extension. + * Idempotent and self-healing for the same reasons as {@link pinBookmark}. * @param {Bookmark} bookmark * @returns {Promise} */ unpinBookmark(bookmark) { - const ns = Strophe.NS.BOOKMARKS_PINNING; - const extensions = (bookmark.get('extensions') ?? []).filter( - /** @param {string} e */ (e) => !(e.includes(' e.includes(', preserves unknown', mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { await mock.waitForRoster(_converse, 'current', 0); await mock.waitUntilBookmarksReturned(_converse); @@ -722,26 +722,30 @@ describe('A bookmark', function () { const muc_jid = 'theplay@conference.shakespeare.lit'; const { state } = _converse; - // A bookmark carrying a malformed extension alongside a valid one. - state.bookmarks.create({ + // Already pinned, plus a malformed extension and an unrelated (unknown) one. + const bookmark = state.bookmarks.create({ jid: muc_jid, autojoin: true, name: 'The Play', nick: 'romeo', - extensions: ['`], + extensions: [ + '', + ``, + ], }); await mock.waitForMUCDiscoInfo(_converse, muc_jid); await u.waitUntil(() => state.chatboxes.length === 1); + expect(bookmark.get('pinned')).toBe(true); const IQ_stanzas = _converse.api.connection.get().IQ_stanzas; - const bookmark = state.bookmarks.findWhere({ jid: muc_jid }); - // Serializing/publishing must not throw: the malformed extension is - // dropped and the valid extension is preserved. (We don't - // await the publish itself, since the mock server never answers the - // IQ; we only care that the stanza was built and sent.) - state.bookmarks.sendBookmarkStanza(bookmark).catch(() => {}); + // Re-pinning must still publish (self-healing if the server diverged), and the + // published must drop the malformed entry, keep exactly one + // (no duplicate) and preserve the unknown extension. (We don't await + // the publish, since the mock server never answers the IQ.) + state.bookmarks.pinBookmark(bookmark).catch(() => {}); const sent_stanza = await u.waitUntil(() => IQ_stanzas.filter( @@ -749,11 +753,49 @@ describe('A bookmark', function () { ).pop() ); - const extensions_els = sizzle('extensions', sent_stanza); - expect(extensions_els.length).toBe(1); - expect(extensions_els[0].children.length).toBe(1); - expect(extensions_els[0].children[0].tagName).toBe('pinned'); - expect(extensions_els[0].children[0].namespaceURI).toBe(Strophe.NS.BOOKMARKS_PINNING); + const children = Array.from(sizzle('extensions', sent_stanza)[0].children); + expect(children.length).toBe(2); // malformed dropped + expect(children.filter((c) => c.namespaceURI === Strophe.NS.BOOKMARKS_PINNING).length).toBe(1); // single + expect(children.filter((c) => c.namespaceURI === 'urn:example:note').length).toBe(1); // unknown preserved + }), + ); + + it( + 'derives pinned-ness by element identity, not substring matching', + mock.initConverse(converse, ['connected', 'chatBoxesFetched'], {}, async function (_converse) { + await mock.waitForRoster(_converse, 'current', 0); + await mock.waitUntilBookmarksReturned(_converse); + + const { state } = _converse; + const ns = Strophe.NS.BOOKMARKS_PINNING; + + // A lookalike the old substring check would have mis-flagged as pinned. + const lookalike = state.bookmarks.create({ + jid: 'a@conference.example', + name: 'A', + autojoin: false, + extensions: [``], + }); + expect(lookalike.get('pinned')).toBe(false); + + // A real with reordered/extra attributes, single quotes and + // an explicit close tag must still count as pinned. + const pinned = state.bookmarks.create({ + jid: 'b@conference.example', + name: 'B', + autojoin: false, + extensions: [``], + }); + expect(pinned.get('pinned')).toBe(true); + + // A nested inside another extension must not count. + const nested = state.bookmarks.create({ + jid: 'c@conference.example', + name: 'C', + autojoin: false, + extensions: [``], + }); + expect(nested.get('pinned')).toBe(false); }), ); }); diff --git a/src/headless/plugins/bookmarks/utils.js b/src/headless/plugins/bookmarks/utils.js index b99752bd48..c575aa20ad 100644 --- a/src/headless/plugins/bookmarks/utils.js +++ b/src/headless/plugins/bookmarks/utils.js @@ -1,9 +1,28 @@ +import { Stanza } from 'strophe.js'; import log from '@converse/log'; import _converse from '../../shared/_converse.js'; import api from '../../shared/api/index.js'; import converse from '../../shared/api/public.js'; -const { u } = converse.env; +const { Strophe, u } = converse.env; + +/** + * Whether a serialized extension string is the XEP-0469 `` element. + * Parses and compares the element's local name and namespace, so it's robust to + * attribute order, whitespace, quoting and namespace-prefix differences — and + * isn't fooled by e.g. `` or a nested `` inside another + * extension. Unparseable strings are treated as "not pinned" (and preserved). + * @param {string} e + * @returns {boolean} + */ +export function isPinnedExtension(e) { + try { + const el = Stanza.toElement(e); + return el.localName === 'pinned' && el.namespaceURI === Strophe.NS.BOOKMARKS_PINNING; + } catch { + return false; + } +} /** * @returns {import('shared/types').StorageKeys} diff --git a/src/headless/types/plugins/bookmarks/collection.d.ts b/src/headless/types/plugins/bookmarks/collection.d.ts index dbe55b2349..081298ff68 100644 --- a/src/headless/types/plugins/bookmarks/collection.d.ts +++ b/src/headless/types/plugins/bookmarks/collection.d.ts @@ -82,9 +82,13 @@ declare class Bookmarks extends Collection { onBookmarksReceivedError(deferred: any, iq: Element): Promise; getUnopenedBookmarks(): Promise; /** - * Pin a bookmark to the top of the lists (XEP-0469) by adding a `` - * element to its extensions. The `pinned` attribute is derived from the - * extensions by {@link Bookmark}, so we only need to update the latter. + * Pin a bookmark to the top of the lists (XEP-0469) by ensuring exactly one + * `` element is present in its extensions. We always (re)publish, + * even when the bookmark already looks pinned locally: the operation is + * idempotent (any existing `` is stripped before re-adding a single + * one, so it can't accumulate duplicates) and self-healing if our local + * state and the server's have diverged. The `pinned` attribute is derived + * from the extensions by {@link Bookmark}. * @param {Bookmark} bookmark * @returns {Promise} */ @@ -98,8 +102,16 @@ declare class Bookmarks extends Collection { * @returns {Promise} */ pinRoom(jid: string): Promise; + /** + * Unpin a room by its JID (XEP-0469). A no-op if the room isn't bookmarked + * (you can only unpin something that was pinned, which requires a bookmark). + * @param {string} jid + * @returns {Promise|void} + */ + unpinRoom(jid: string): Promise | void; /** * Unpin a bookmark (XEP-0469) by removing its `` extension. + * Idempotent and self-healing for the same reasons as {@link pinBookmark}. * @param {Bookmark} bookmark * @returns {Promise} */ diff --git a/src/headless/types/plugins/bookmarks/utils.d.ts b/src/headless/types/plugins/bookmarks/utils.d.ts index bfb6e0dabd..82d519d3e9 100644 --- a/src/headless/types/plugins/bookmarks/utils.d.ts +++ b/src/headless/types/plugins/bookmarks/utils.d.ts @@ -1,3 +1,13 @@ +/** + * Whether a serialized extension string is the XEP-0469 `` element. + * Parses and compares the element's local name and namespace, so it's robust to + * attribute order, whitespace, quoting and namespace-prefix differences — and + * isn't fooled by e.g. `` or a nested `` inside another + * extension. Unparseable strings are treated as "not pinned" (and preserved). + * @param {string} e + * @returns {boolean} + */ +export function isPinnedExtension(e: string): boolean; /** * @returns {import('shared/types').StorageKeys} */ diff --git a/src/plugins/roomslist/view.js b/src/plugins/roomslist/view.js index 62aec3ee74..7241b6180c 100644 --- a/src/plugins/roomslist/view.js +++ b/src/plugins/roomslist/view.js @@ -100,10 +100,7 @@ export class RoomsList extends CustomElement { ev.preventDefault(); const target = /** @type {HTMLElement} */ (ev.currentTarget); const jid = target.getAttribute('data-room-jid'); - const { bookmarks } = _converse.state; - bookmarks - .where({ jid }) - .forEach((b) => bookmarks.unpinBookmark(b)); + _converse.state.bookmarks.unpinRoom(jid); } /** @param {Event} [ev] */