1069845 - Viewing logs from faceted search results is broken

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[aleth [:aleth]]

STR
Search for some string that has hits in chat logs.
Click on a chat log.
The desired log does not open, instead a broken search result pane appears.

Timestamp: 19/09/2014 00:13:26
Error: A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Fri Sep 19 2014 00:13:21 GMT+0200 (CEST)
Full Message: Error: Log was passed an invalid argument, expected a non-empty array or a string.
Source File: /dist/Daily.app/Contents/MacOS/components/logger.js
Line: 449
Source Code:
449

I really hope this is not already broken in TB 31.

Comment 1

[aleth [:aleth]]

Probably a dupe?

Comment 2

[aleth [:aleth]]

While adding the unread ruler to TB, I noticed that this code looks like it hasn't been touched in a long time:

https://dxr.mozilla.org/comm-central/source/mail/components/im/content/chat-messenger-overlay.js#117

Probably a good place to start digging.

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch

This is a very unfortunate regression caused by bug 955292.
The index_im changes from that bug cause chat logs to be indexed against their full paths, instead of path relative to the logs directory.
This patch fixes that, but we now need a way to re-index chat log files since bug 955292 landed and remove the now-garbage entries. I'll file a follow-up for that.
Thankfully, release channel users won't be affected.

Comment 4

[Kent James (:rkent)]

When did this regression first land in Thunderbird?

Comment 5

[aleth [:aleth]]

Comment on attachment 8567381 [details] [diff] [review]
Patch

Review of attachment 8567381 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this! Unfortunately tidying up the effects of the regression is likely going to be more complicated...

::: mail/components/im/content/chat-messenger-overlay.js
@@ +606,3 @@
>        imServices.logs.getLogFromFile(path, true).then(aLog => {
>          aLog.getConversation().then(aConv => {
>            this._showLog(aConv, path, item.searchTerm || undefined);

I was going to say this should be aLog.path not path, but in fact the path argument of _showLog is never used for anything anymore. Since I made the change that caused that, I'll file a followup to clean this up.

Comment 6

[aleth [:aleth]]

(In reply to Kent James (:rkent) from comment #4)
> When did this regression first land in Thunderbird?

This July 4 push caused the regression https://hg.mozilla.org/comm-central/pushloghtml?changeset=1c2ff401d92a, which puts it in TB 33. I have checked the ESR 31.4 is not affected.

Comment 7

[aleth [:aleth]]

As landing this will cause further breakage for users of TB 33 and up, let's hold off landing until the migration patch is ready if we can.

Comment 8

[aleth [:aleth]]

It's a day before the merge, so I'm pushing this.

Comment 9

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/997b38098ace