Closed
Bug 777873
Opened 12 years ago
Closed 12 years ago
Viewing some old Twitter logs breaks log viewer
Categories
(Thunderbird :: Instant Messaging, defect)
Tracking
(thunderbird15 fixed, thunderbird16 fixed)
RESOLVED
FIXED
Thunderbird 17.0
People
(Reporter: mconley, Assigned: florian)
Details
Attachments
(1 file)
5.04 KB,
patch
|
clokep
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
STR: In the Chat tab, choose a previous conversation from several weeks ago What happens? The log is not displayed. What's more, viewing *any* logs at this point is broken. What's expected? The log viewer should work.
Reporter | ||
Comment 1•12 years ago
|
||
This appears in the Error Console: Timestamp: 26/07/2012 3:58:01 PM Error: TypeError: aConv is null Source File: chrome://chat/content/convbrowser.xml Line: 713
Reporter | ||
Comment 3•12 years ago
|
||
(I'm unsure how critical an error this is - perhaps my old logs are corrupt somehow...they go all the way back to April 1st. Regardless, we've clearly got a hole in our error handling here.)
Assignee | ||
Comment 4•12 years ago
|
||
This patch handles 3 error cases: - log files that aren't in the JSON format (I think Mike may have tested a very old try server build of IM in Tb that still wrote plain text logs. The error in the console in that case is exactly what Mike reported) - random files in the log folder that don't have a name matching the expected format (these files are now just ignored, instead of breaking the list of logs) - random junk inside a log file with a correct name and a .json extension. (The log file will be ignored if the first line is not valid JSON. If the junk starts later, only the lines that aren't valid JSON will be ignored).
Assignee: nobody → florian
Attachment #650187 -
Flags: review?(clokep)
Comment 5•12 years ago
|
||
Comment on attachment 650187 [details] [diff] [review] Patch (In reply to Florian Quèze [:florian] [:flo] from comment #4) > This patch handles 3 error cases: > - log files that aren't in the JSON format (I think Mike may have tested a > very old try server build of IM in Tb that still wrote plain text logs. The > error in the console in that case is exactly what Mike reported) Shouldn't these go into the text log reader we have then or am I misunderstanding that? The changes here look fine though!
Attachment #650187 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Patrick Cloke [:clokep] from comment #5) > > - log files that aren't in the JSON format (I think Mike may have tested a > > very old try server build of IM in Tb that still wrote plain text logs. The > > error in the console in that case is exactly what Mike reported) > > Shouldn't these go into the text log reader we have then or am I > misunderstanding that? Thunderbird never logged in the plain text format (except for a few very early testing builds) so it doesn't have the text log reader that Instantbird has.
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 650187 [details] [diff] [review] Patch [Approval Request Comment] User impact if declined: parts of the chat UI will be broken if the user has random junk in a log folder. Risk to taking this patch: low. Should we take this for Tb15? I really don't know. The risk is low (the patch mostly adds null checks and try/catch in a few places), but the benefit seems only marginal (ie, why would users have random invalid log files in their log folders?). I'm requesting approval mostly so that someone actually make a decision here.
Attachment #650187 -
Flags: approval-comm-beta?
Attachment #650187 -
Flags: approval-comm-aurora?
Comment 8•12 years ago
|
||
Comment on attachment 650187 [details] [diff] [review] Patch Although there's probably not too many people with corrupted logs, I think keeping working if we get into that situation is probably a good idea. We don't know yet the full extent of what we'll hit, so lets take it and have less risk there.
Attachment #650187 -
Flags: approval-comm-beta?
Attachment #650187 -
Flags: approval-comm-beta+
Attachment #650187 -
Flags: approval-comm-aurora?
Attachment #650187 -
Flags: approval-comm-aurora+
Reporter | ||
Comment 9•12 years ago
|
||
comm-central: https://hg.mozilla.org/comm-central/rev/4d1d1564495a comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/f1d65ccc71a9 comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/728b2c0854a4
Status: NEW → RESOLVED
Closed: 12 years ago
status-thunderbird15:
--- → fixed
status-thunderbird16:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
Assignee | ||
Comment 10•12 years ago
|
||
The chat/ part landed for Instantbird as http://hg.instantbird.org/instantbird/rev/8ec7e45f71fd and the UI changes were dealt with in https://bugzilla.instantbird.org/show_bug.cgi?id=1448
You need to log in
before you can comment on or make changes to this bug.
Description
•