diff --git a/play.pokemonshowdown.com/src/battle-log.ts b/play.pokemonshowdown.com/src/battle-log.ts index 7f7b9cee4cb..b719b2e2b25 100644 --- a/play.pokemonshowdown.com/src/battle-log.ts +++ b/play.pokemonshowdown.com/src/battle-log.ts @@ -55,7 +55,7 @@ export class BattleLog { * * 1 = player 2: "Red sent out Pikachu!" "Eevee used Tackle!" */ perspective: -1 | 0 | 1 = -1; - getHighlight: ((line: Args) => boolean) | null = null; + getHighlight: ((line: Args, isHistory?: boolean) => boolean) | null = null; constructor(elem: HTMLDivElement, scene?: BattleScene | null, innerElem?: HTMLDivElement) { this.elem = elem; @@ -122,7 +122,7 @@ export class BattleLog { }); this.addNode(el); } - add(args: Args, kwArgs?: KWArgs, preempt?: boolean, showTimestamps?: 'minutes' | 'seconds') { + add(args: Args, kwArgs?: KWArgs, preempt?: boolean, showTimestamps?: 'minutes' | 'seconds', isHistory?: boolean) { if (kwArgs?.silent) return; const battle = this.scene?.battle; if (battle?.seeking) { @@ -178,7 +178,7 @@ export class BattleLog { } timestampHtml = `[${components.map(x => x < 10 ? `0${x}` : x).join(':')}] `; } - const isHighlighted = window.app?.rooms?.[battle!.roomid].getHighlight(message) || this.getHighlight?.(args); + const isHighlighted = window.app?.rooms?.[battle!.roomid].getHighlight(message) || this.getHighlight?.(args, isHistory); [divClass, divHTML, noNotify] = this.parseChatMessage(message, name, timestampHtml, isHighlighted); if (!noNotify && isHighlighted) { const notifyTitle = "Mentioned by " + name + " in " + (battle?.roomid || ''); diff --git a/play.pokemonshowdown.com/src/panel-chat.tsx b/play.pokemonshowdown.com/src/panel-chat.tsx index 4869a7bc97c..4b6c9d726b1 100644 --- a/play.pokemonshowdown.com/src/panel-chat.tsx +++ b/play.pokemonshowdown.com/src/panel-chat.tsx @@ -22,6 +22,22 @@ import { ChatTournament, TournamentBox } from "./panel-chat-tournament"; declare const formatText: any; // from js/server/chat-formatter.js +export function shouldNotifyHighlight({ + isHistory, + authorUserid, + currentUserid, + highlightMatches, +}: { + isHistory: boolean, + authorUserid: ID, + currentUserid: ID, + highlightMatches: boolean, +}): boolean { + if (authorUserid === currentUserid) return false; + if (isHistory) return false; + return highlightMatches; +} + type Challenge = { formatName: string, teamFormat: string, @@ -272,7 +288,7 @@ export class ChatRoom extends PSRoom { this.highlightRegExp[i] = new RegExp('(?:\\b|(?!\\w))(?:' + highlights[i].join('|') + ')(?:\\b|(?!\\w))', 'i'); } } - handleHighlight = (args: Args) => { + handleHighlight = (args: Args, isHistory?: boolean) => { let name; let message; let serverTime = 0; @@ -284,11 +300,24 @@ export class ChatRoom extends PSRoom { name = args[1]; message = args[2]; } - if (toID(name) === PS.user.userid) return false; if (message.startsWith(`/raw `) || message.startsWith(`/uhtml`) || message.startsWith(`/uhtmlchange`)) { return false; } + // check highlight match + const highlightMatches = !!ChatRoom.getHighlight(message, this.id); + if (!highlightMatches) return false; + + const shouldNotify = shouldNotifyHighlight({ + isHistory: !!isHistory, + authorUserid: toID(name), + currentUserid: PS.user.userid, + highlightMatches, + }); + + // history: return true for css but skip notify + if (!shouldNotify) return highlightMatches; + const lastMessageDates = Dex.prefs('logtimes') || (PS.prefs.set('logtimes', {}), Dex.prefs('logtimes')); if (!lastMessageDates[PS.server.id]) lastMessageDates[PS.server.id] = {}; const lastMessageDate = lastMessageDates[PS.server.id][this.id] || 0; @@ -303,16 +332,13 @@ export class ChatRoom extends PSRoom { const lastMessageTime = this.lastMessageTime || 0; if (lastMessageTime < time) this.lastMessageTime = time; } - if (ChatRoom.getHighlight(message, this.id)) { - const mayNotify = time > lastMessageDate; - if (mayNotify) this.notify({ - title: `Mentioned by ${name} in ${this.id}`, - body: `"${message}"`, - id: 'highlight', - }); - return true; - } - return false; + const mayNotify = time > lastMessageDate; + if (mayNotify) this.notify({ + title: `Mentioned by ${name} in ${this.id}`, + body: `"${message}"`, + id: 'highlight', + }); + return true; }; override clientCommands = this.parseClientCommands({ 'chall,challenge'(target) { @@ -1444,7 +1470,7 @@ export class ChatLog extends preact.Component<{ const backlog = room.backlog; room.backlog = null; for (const args of backlog) { - room.log.add(args, undefined, undefined, PS.prefs.timestamps[room.pmTarget ? 'pms' : 'chatrooms']); + room.log.add(args, undefined, undefined, PS.prefs.timestamps[room.pmTarget ? 'pms' : 'chatrooms'], true); } } this.subscription = room.subscribe(tokens => { diff --git a/test/highlight.test.js b/test/highlight.test.js new file mode 100644 index 00000000000..0917882dfdf --- /dev/null +++ b/test/highlight.test.js @@ -0,0 +1,91 @@ +const assert = require('assert').strict; + +// test implementation matching panel-chat.tsx logic +function shouldNotifyHighlight({ + isHistory, + authorUserid, + currentUserid, + highlightMatches, +}) { + if (authorUserid === currentUserid) return false; + if (isHistory) return false; + return highlightMatches; +} + +describe('Highlight Notifications', () => { + + describe('shouldNotifyHighlight', () => { + + it('should not notify for history/backlog messages even when highlight matches', () => { + const result = shouldNotifyHighlight({ + isHistory: true, + authorUserid: 'otheruser', + currentUserid: 'testuser', + highlightMatches: true, + }); + assert.strictEqual(result, false); + }); + + it('should not notify for self-authored messages even when highlight matches', () => { + const result = shouldNotifyHighlight({ + isHistory: false, + authorUserid: 'testuser', + currentUserid: 'testuser', + highlightMatches: true, + }); + assert.strictEqual(result, false); + }); + + it('should not notify for self-authored history messages', () => { + const result = shouldNotifyHighlight({ + isHistory: true, + authorUserid: 'testuser', + currentUserid: 'testuser', + highlightMatches: true, + }); + assert.strictEqual(result, false); + }); + + it('should notify for live messages from other users when highlight matches', () => { + const result = shouldNotifyHighlight({ + isHistory: false, + authorUserid: 'otheruser', + currentUserid: 'testuser', + highlightMatches: true, + }); + assert.strictEqual(result, true); + }); + + it('should not notify for live messages when highlight does not match', () => { + const result = shouldNotifyHighlight({ + isHistory: false, + authorUserid: 'otheruser', + currentUserid: 'testuser', + highlightMatches: false, + }); + assert.strictEqual(result, false); + }); + + it('should not notify when current user is not yet known (empty userid)', () => { + const result = shouldNotifyHighlight({ + isHistory: false, + authorUserid: 'someuser', + currentUserid: '', + highlightMatches: true, + }); + assert.strictEqual(result, true); + }); + + it('should not notify for history when current user is not yet known', () => { + const result = shouldNotifyHighlight({ + isHistory: true, + authorUserid: 'someuser', + currentUserid: '', + highlightMatches: true, + }); + assert.strictEqual(result, false); + }); + + }); + +});