Skip to content

Commit 7b4e3cd

Browse files
jcbrandjcbrand
committed
Use retract element to remove bookmarks
Fixes #3815
1 parent 04bce4d commit 7b4e3cd

File tree

5 files changed

+191
-27
lines changed

5 files changed

+191
-27
lines changed

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Next release
44

5+
- #3815: Removal of boomark leads to new bookmark named `Symbol(lit-nothing)`
56
- #3824: Dates and times are not translated
67
- #3829: Rich text from LibreOffice Calc is sent as screenshots
78
- #3830: The message textarea blocks undo of the pasted text

‎src/headless/plugins/bookmarks/collection.js‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,13 @@ class Bookmarks extends Collection {
3636
.then((bm) => this.markRoomAsBookmarked(bm))
3737
.catch((e) => log.fatal(e))
3838
);
39-
this.on('remove', this.leaveRoom, this);
4039
this.on('change:autojoin', this.onAutoJoinChanged, this);
4140
this.on(
4241
'remove',
43-
/** @param {Bookmark} bookmarks */
44-
async (bookmark, bookmarks) => {
45-
const bare_jid = _converse.session.get('bare_jid');
46-
const sourceBookmark = (await api.disco.supports(`${Strophe.NS.BOOKMARKS2}#compat`, bare_jid))
47-
? bookmark
48-
: bookmarks;
49-
this.removeBookmarkStanza(sourceBookmark);
50-
},
51-
this
42+
/** @param { Bookmark } bookmark }*/ (bookmark) => {
43+
this.sendRemoveBookmarkStanza(bookmark);
44+
this.leaveRoom(bookmark);
45+
}
5246
);
5347

5448
const { session } = _converse;
@@ -141,32 +135,39 @@ class Bookmarks extends Collection {
141135
* @param {Bookmark} bookmark
142136
* @returns {Promise<void|Element>}
143137
*/
144-
async removeBookmarkStanza(bookmark) {
138+
async sendRemoveBookmarkStanza(bookmark) {
145139
const bare_jid = _converse.session.get('bare_jid');
146-
147140
const node = (await api.disco.supports(`${Strophe.NS.BOOKMARKS2}#compat`, bare_jid))
148141
? Strophe.NS.BOOKMARKS2
149142
: Strophe.NS.BOOKMARKS;
150143

151144
if (node === Strophe.NS.BOOKMARKS2) {
152-
const retractItem = stx`
153-
<item id="${bookmark.get('jid')}">
154-
<remove/>
155-
</item>`;
156-
157-
return api.pubsub.publish(null, node, retractItem, { persist_items: true });
145+
const stanza = stx`
146+
<iq from="${bare_jid}"
147+
to="${bare_jid}"
148+
type="set"
149+
xmlns="jabber:client">
150+
<pubsub xmlns="http://jabber.org/protocol/pubsub">
151+
<retract node="${node}" notify="true">
152+
<item id="${bookmark.get('jid')}"/>
153+
</retract>
154+
</pubsub>
155+
</iq>`;
156+
return api.sendIQ(stanza);
158157
}
159158

160-
return this.sendBookmarkStanza(bookmark);
159+
return this.sendBookmarkStanza().catch((iq) => this.onBookmarkError(iq));
161160
}
162161

163162
/**
164163
* @param {'urn:xmpp:bookmarks:1'|'storage:bookmarks'} node
165-
* @param {Bookmark} bookmark
164+
* @param {Bookmark} [bookmark]
166165
* @returns {Stanza|Stanza[]}
167166
*/
168167
getPublishedItems(node, bookmark) {
169168
if (node === Strophe.NS.BOOKMARKS2) {
169+
if (!bookmark) throw new Error('getPublishedItems: missing bookmark');
170+
170171
const extensions = bookmark.get('extensions') ?? [];
171172
return stx`<item id="${bookmark.get('jid')}">
172173
<conference xmlns="${Strophe.NS.BOOKMARKS2}"
@@ -198,7 +199,7 @@ class Bookmarks extends Collection {
198199
}
199200

200201
/**
201-
* @param {Bookmark} bookmark
202+
* @param {Bookmark} [bookmark]
202203
* @returns {Promise<void|Element>}
203204
*/
204205
async sendBookmarkStanza(bookmark) {
@@ -219,7 +220,7 @@ class Bookmarks extends Collection {
219220
* @param {Element} iq
220221
*/
221222
onBookmarkError(iq) {
222-
log.error('Error while trying to add bookmark');
223+
log.error('Error while trying to update bookmarks');
223224
log.error(iq);
224225
}
225226

‎src/headless/plugins/bookmarks/tests/bookmarks.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,4 +468,65 @@ describe("A bookmark", function () {
468468
expect(result).toBe(true);
469469
})
470470
);
471+
472+
it(
473+
'can be removed and sends out a retract stanza',
474+
mock.initConverse(['connected', 'chatBoxesFetched'], {}, async function (_converse) {
475+
await mock.waitForRoster(_converse, 'current', 0);
476+
await mock.waitUntilDiscoConfirmed(
477+
_converse,
478+
_converse.bare_jid,
479+
[{ 'category': 'pubsub', 'type': 'pep' }],
480+
['http://jabber.org/protocol/pubsub#publish-options', 'urn:xmpp:bookmarks:1#compat']
481+
);
482+
await mock.waitUntilBookmarksReturned(_converse);
483+
484+
const bare_jid = _converse.session.get('bare_jid');
485+
const muc_jid = '[email protected]';
486+
const { api } = _converse;
487+
488+
// First create a bookmark
489+
await api.bookmarks.set({
490+
jid: muc_jid,
491+
autojoin: true,
492+
name: 'The Play',
493+
nick: 'romeo',
494+
});
495+
496+
const IQ_stanzas = _converse.api.connection.get().IQ_stanzas;
497+
let sent_stanza = await u.waitUntil(() =>
498+
IQ_stanzas.filter((s) => sizzle('publish[node="urn:xmpp:bookmarks:1"]', s).length).pop()
499+
);
500+
501+
// Server acknowledges successful storage
502+
const result_stanza = stx`
503+
<iq xmlns="jabber:client"
504+
to="${_converse.api.connection.get().jid}"
505+
type="result"
506+
id="${sent_stanza.getAttribute('id')}"/>`;
507+
_converse.api.connection.get()._dataRecv(mock.createRequest(result_stanza));
508+
509+
// Clear previous stanzas
510+
while (IQ_stanzas.length) IQ_stanzas.pop();
511+
512+
// Now remove the bookmark
513+
const bookmark = _converse.state.bookmarks.findWhere({ jid: muc_jid });
514+
expect(bookmark).toBeTruthy();
515+
_converse.state.bookmarks.remove(bookmark);
516+
517+
// Check that a retract stanza is sent as per XEP-0402
518+
sent_stanza = await u.waitUntil(() =>
519+
IQ_stanzas.filter((s) => sizzle('retract[node="urn:xmpp:bookmarks:1"]', s).length).pop()
520+
);
521+
522+
expect(sent_stanza).toEqualStanza(stx`
523+
<iq from="${bare_jid}" to="${bare_jid}" id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">
524+
<pubsub xmlns="http://jabber.org/protocol/pubsub">
525+
<retract node="urn:xmpp:bookmarks:1" notify="true">
526+
<item id="${muc_jid}"/>
527+
</retract>
528+
</pubsub>
529+
</iq>`);
530+
})
531+
);
471532
});

‎src/headless/plugins/bookmarks/tests/deprecated.js‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,105 @@ describe("A bookmark", function () {
257257
</pubsub>
258258
</iq>`);
259259
}));
260+
261+
it("can be removed and republishes all remaining bookmarks as per XEP-0048", mock.initConverse(
262+
['connected', 'chatBoxesFetched'], {}, async function (_converse) {
263+
264+
await mock.waitForRoster(_converse, 'current', 0);
265+
await mock.waitUntilBookmarksReturned(
266+
_converse,
267+
[],
268+
['http://jabber.org/protocol/pubsub#publish-options', 'http://jabber.org/protocol/pubsub#config-node-max'],
269+
'storage:bookmarks'
270+
);
271+
272+
const bare_jid = _converse.session.get('bare_jid');
273+
const muc1_jid = '[email protected]';
274+
const muc2_jid = '[email protected]';
275+
const { bookmarks } = _converse.state;
276+
277+
// First create two bookmarks
278+
bookmarks.setBookmark({
279+
jid: muc1_jid,
280+
autojoin: true,
281+
name: 'Hamlet',
282+
nick: ''
283+
});
284+
285+
const IQ_stanzas = _converse.api.connection.get().IQ_stanzas;
286+
let sent_stanza = await u.waitUntil(
287+
() => IQ_stanzas.filter(s => sizzle('item[id="current"]', s).length).pop());
288+
289+
// Server acknowledges successful storage
290+
const result_stanza = stx`
291+
<iq xmlns="jabber:client"
292+
to="${_converse.api.connection.get().jid}"
293+
type="result"
294+
id="${sent_stanza.getAttribute('id')}"/>`;
295+
_converse.api.connection.get()._dataRecv(mock.createRequest(result_stanza));
296+
297+
bookmarks.setBookmark({
298+
jid: muc2_jid,
299+
autojoin: true,
300+
name: 'Balcony',
301+
nick: 'romeo'
302+
});
303+
304+
sent_stanza = await u.waitUntil(
305+
() => IQ_stanzas.filter(s => sizzle('item[id="current"] conference[name="Balcony"]', s).length).pop());
306+
307+
// Server acknowledges successful storage
308+
const result_stanza2 = stx`
309+
<iq xmlns="jabber:client"
310+
to="${_converse.api.connection.get().jid}"
311+
type="result"
312+
id="${sent_stanza.getAttribute('id')}"/>`;
313+
_converse.api.connection.get()._dataRecv(mock.createRequest(result_stanza2));
314+
315+
// Clear previous stanzas
316+
while (IQ_stanzas.length) { IQ_stanzas.pop(); }
317+
318+
// Now remove one bookmark
319+
const bookmark = bookmarks.findWhere({jid: muc1_jid});
320+
expect(bookmark).toBeTruthy();
321+
bookmarks.remove(bookmark);
322+
323+
// Check that a stanza is sent with all remaining bookmarks (XEP-0048 style)
324+
sent_stanza = await u.waitUntil(
325+
() => IQ_stanzas.filter(s => sizzle('publish[node="storage:bookmarks"]', s).length).pop());
326+
327+
expect(sent_stanza).toEqualStanza(stx`
328+
<iq from="${bare_jid}" to="${bare_jid}" id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">
329+
<pubsub xmlns="http://jabber.org/protocol/pubsub">
330+
<publish node="storage:bookmarks">
331+
<item id="current">
332+
<storage xmlns="storage:bookmarks">
333+
<conference autojoin="true" jid="${muc2_jid}" name="Balcony">
334+
<nick>romeo</nick>
335+
</conference>
336+
</storage>
337+
</item>
338+
</publish>
339+
<publish-options>
340+
<x type="submit" xmlns="jabber:x:data">
341+
<field type="hidden" var="FORM_TYPE">
342+
<value>http://jabber.org/protocol/pubsub#publish-options</value>
343+
</field>
344+
<field var='pubsub#persist_items'>
345+
<value>true</value>
346+
</field>
347+
<field var='pubsub#max_items'>
348+
<value>max</value>
349+
</field>
350+
<field var='pubsub#send_last_published_item'>
351+
<value>never</value>
352+
</field>
353+
<field var='pubsub#access_model'>
354+
<value>whitelist</value>
355+
</field>
356+
</x>
357+
</publish-options>
358+
</pubsub>
359+
</iq>`);
360+
}));
260361
});

‎src/headless/types/plugins/bookmarks/collection.d.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,18 @@ declare class Bookmarks extends Collection<Bookmark> {
3131
* @param {Bookmark} bookmark
3232
* @returns {Promise<void|Element>}
3333
*/
34-
removeBookmarkStanza(bookmark: Bookmark): Promise<void | Element>;
34+
sendRemoveBookmarkStanza(bookmark: Bookmark): Promise<void | Element>;
3535
/**
3636
* @param {'urn:xmpp:bookmarks:1'|'storage:bookmarks'} node
37-
* @param {Bookmark} bookmark
37+
* @param {Bookmark} [bookmark]
3838
* @returns {Stanza|Stanza[]}
3939
*/
40-
getPublishedItems(node: "urn:xmpp:bookmarks:1" | "storage:bookmarks", bookmark: Bookmark): Stanza | Stanza[];
40+
getPublishedItems(node: "urn:xmpp:bookmarks:1" | "storage:bookmarks", bookmark?: Bookmark): Stanza | Stanza[];
4141
/**
42-
* @param {Bookmark} bookmark
42+
* @param {Bookmark} [bookmark]
4343
* @returns {Promise<void|Element>}
4444
*/
45-
sendBookmarkStanza(bookmark: Bookmark): Promise<void | Element>;
45+
sendBookmarkStanza(bookmark?: Bookmark): Promise<void | Element>;
4646
/**
4747
* @param {Element} iq
4848
*/

0 commit comments

Comments
 (0)