Viewing logs from faceted search results is broken

RESOLVED FIXED in Thunderbird 38.0

Status

defect
--
major
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: aleth, Assigned: nhnt11)

Tracking

Trunk
Thunderbird 38.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

4 years ago
Probably a dupe?
(Reporter)

Updated

4 years ago
Depends on: 1103647
(Reporter)

Comment 2

4 years ago
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.
(Assignee)

Comment 3

4 years ago
Posted patch PatchSplinter Review
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.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Attachment #8567381 - Flags: review?(aleth)
(Assignee)

Updated

4 years ago
Depends on: 1135291
When did this regression first land in Thunderbird?
(Reporter)

Comment 5

4 years ago
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.
Attachment #8567381 - Flags: review?(aleth) → review+
(Reporter)

Comment 6

4 years ago
(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.
(Reporter)

Comment 7

4 years ago
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.
(Reporter)

Updated

4 years ago
Blocks: 1135297
(Reporter)

Comment 8

4 years ago
It's a day before the merge, so I'm pushing this.
(Reporter)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
(Reporter)

Updated

4 years ago
You need to log in before you can comment on or make changes to this bug.