Closed
Bug 954882
Opened 10 years ago
Closed 10 years ago
Handle bad log files properly
Categories
(Instantbird Graveyard :: Other, defect)
Instantbird Graveyard
Other
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: aleth, Assigned: aleth)
References
Details
Attachments
(1 file, 5 obsolete files)
9.97 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1448 at 2012-05-20 16:13:00 UTC *** Bad log files are not so rare (due to system crashes, power outages, and the like). Currently IB does two things 1) Throw an exception ('bad log file') and subsequent errors due to there being no file. 2) Include the bad log file in the list of log files, but display some other log (the previous or following log) when selecting this file. This is not good. Instead, either the log file should not be included in the list at all, or (better I think, because then there is a record that a conversation existed at that time) a "Corrupt log file" message should be displayed in the log view browser when it is selected.
Comment 2•10 years ago
|
||
*** Original post on bio 1448 at 2012-08-14 14:25:47 UTC *** Is this essentially https://bugzilla.mozilla.org/show_bug.cgi?id=777873
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1835 at 2012-08-24 23:09:00 UTC *** The /chat part (logger.js) matches the TB commit. The log viewer part takes account of the fact that IB users may have text logs, and shows an error page when a JSON log is corrupt or empty. I think this is preferable to suppressing such a log completely - the fact that it has the json ending makes it likely that it was created by IB, and so it is better if the user finds this out and doesn't spend ages trying to find a "missing" log. (Corrupt lines within otherwise correct JSON files are simply ignored on reading as per the TB patch, but that's different from the case where a log file is otherwise missing altogether.)
Attachment #8353594 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1836 at 2012-08-24 23:23:00 UTC *** Removed an unwanted change.
Attachment #8353595 -
Flags: review?(florian)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8353594 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1835 at 2012-08-24 23:23:37 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353594 -
Attachment is obsolete: true
Attachment #8353594 -
Flags: review?(florian)
Comment 6•10 years ago
|
||
*** Original post on bio 1448 at 2012-08-24 23:33:55 UTC *** I'm a bit surprised that there's no CSS change included in the patch. Wouldn't we want the error message to be displayed like the "no account... blahblahblah" place holder of the account manager?
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1838 at 2012-08-25 00:40:00 UTC *** Patch with prettier error message
Attachment #8353597 -
Flags: review?(florian)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8353595 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1836 at 2012-08-25 00:40:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353595 -
Attachment is obsolete: true
Attachment #8353595 -
Flags: review?(florian)
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1839 at 2012-08-25 00:42:00 UTC *** Fix indentation error (sorry)
Attachment #8353598 -
Flags: review?(florian)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8353597 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1838 at 2012-08-25 00:42:42 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353597 -
Attachment is obsolete: true
Attachment #8353597 -
Flags: review?(florian)
Assignee | ||
Comment 11•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1840 at 2012-08-25 00:52:00 UTC *** Handle empty text log files too for consistency.
Attachment #8353599 -
Flags: review?(florian)
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8353598 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1839 at 2012-08-25 00:52:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353598 -
Attachment is obsolete: true
Attachment #8353598 -
Flags: review?(florian)
Assignee | ||
Comment 13•10 years ago
|
||
*** Original post on bio 1448 as attmnt 1841 at 2012-08-25 01:12:00 UTC *** Ensure error message can wordwrap (just in case some l10n is verbose). Hopefully the last nit ;)
Attachment #8353600 -
Flags: review?(florian)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8353599 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1840 at 2012-08-25 01:12:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353599 -
Attachment is obsolete: true
Attachment #8353599 -
Flags: review?(florian)
Comment 15•10 years ago
|
||
Comment on attachment 8353600 [details] [diff] [review] Patch *** Original change on bio 1448 attmnt 1841 at 2012-09-20 16:04:50 UTC *** >diff --git a/chrome/instantbird/skin/classic/instantbird/viewlog.css b/chrome/instantbird/skin/classic/instantbird/viewlog.css >+#corruptLogImage { >+ list-style-image: url("chrome://global/skin/icons/error-48.png"); Unfortunately this file doesn't exist on Mac. I added an ifdef to use error-large.png on Mac instead. My only other comment is that I think it would have been better to hide the findbar when the error message is displayed, rather than making it point at a hidden about:blank document, but this has already received more effort than (I think) it was really worth, so I won't ask you to make any additional change. Thanks for taking care of these error cases, and sorry the review took so long.
Attachment #8353600 -
Flags: review?(florian) → review+
Comment 16•10 years ago
|
||
*** Original post on bio 1448 at 2012-09-20 16:08:11 UTC *** http://hg.instantbird.org/instantbird/rev/16d72d5a3089
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•