Closed Bug 1280824 Opened 4 years ago Closed 4 years ago

Thunderbird freezes on Convert / Event

Categories

(Calendar :: General, defect)

Lightning 4.0.8
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcepl, Assigned: merike)

References

Details

Attachments

(3 files)

Attached file backtrace
When trying to convert a message (without any date in the body of text) to an Event, Thunderbird freezes even with the context menu opened, and it doesn't seem to ever finish. See attached backtrace and notice especially line:

[calExtract] Faulty extraction pattern do %1$S:%2$S | do %1$S hodin %2$S | - %1$S:%2$S | - %1$S hodin %2$S | a %1$S:%2$S | a %1$S hodin %2$S | for until.hour.minutes

Using thunderbird-38.8.0-1.el7_2.x86_64 from RHEL-7 repos.
Attached file test message
This is a mistake in locale's patterns which are not supposed to have | in the end of a pattern. I'd forward it to Greek localizers but according to http://hg.mozilla.org/releases/l10n/mozilla-aurora/el/file/tip/calendar/chrome/calendar/calendar-extract.properties they don't have patterns localized at all currently.

I believe in the past this error resulted in the event dialog not opening due to the error but not in freezing Thunderbird. Is it just the context menu that remains open or can't you use Thunderbird at all after trying this?

Technically we could also handle the empty pattern in code but this would make it less likely to get such locale errors fixed as no one constantly monitors error console while things seem to be working fine.
(In reply to Merike (:merike) from comment #2)
> This is a mistake in locale's patterns which are not supposed to have | in
> the end of a pattern. I'd forward it to Greek localizers but according to
> http://hg.mozilla.org/releases/l10n/mozilla-aurora/el/file/tip/calendar/
> chrome/calendar/calendar-extract.properties they don't have patterns
> localized at all currently.

Greek? I am Czech, so it should be either Czech or English locale we are talking about (I have desktop in English).
In any case, the code should not cause freezing. If you want to show a big error in the error console that is fine, but it would be nice to unfreeze it if there is an error.
Does the issue still exists using the latest release (Thunderbird 45.x with Lightning 4.7.x)?
Flags: needinfo?(mcepl)
(In reply to Stefan Sitter from comment #5)
> Does the issue still exists using the latest release (Thunderbird 45.x with
> Lightning 4.7.x)?

Oh well, yes, I cannot reproduce it with TB 45, so I am out of luck, right?
Flags: needinfo?(mcepl)
(In reply to Matěj Cepl from comment #3)
> Greek? I am Czech, so it should be either Czech or English locale we are
> talking about (I have desktop in English).
Sorry, I read your report too fast too late in the evening and mistook the "el" within "thunderbird-38.8.0-1.el7_2.x86_64" to mean a locale code.

(In reply to Matěj Cepl from comment #6)
> Oh well, yes, I cannot reproduce it with TB 45, so I am out of luck, right?
This is because Czech has fixed it in https://hg.mozilla.org/releases/l10n/mozilla-aurora/cs/rev/5234666339b8

I'm going to check if I can see the freeze myself with the faulty pattern still present and report back. Not opening event dialog in such case is one thing but freeze like you described is not acceptable.
Actually I can indeed as the code only reports the error but doesn't stop executing here: http://hg.mozilla.org/comm-central/file/tip/calendar/base/modules/calExtract.jsm#l1118

I can probably take a look at what's a good way to eliminate this freeze from occurring later this week. I think ignoring empty variants that checkForFaultyPatterns is checking for may be the way to go.
Attached patch emptyPatternSplinter Review
This should make extraction ignore empty variants causing freezes. Error is still logged.
Attachment #8766520 - Flags: review?(philipp)
Assignee: nobody → merikes.lists
Status: NEW → ASSIGNED
Comment on attachment 8766520 [details] [diff] [review]
emptyPattern

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

::: calendar/base/modules/calExtract.jsm
@@ +994,5 @@
>              let vals = this.cleanPatterns(value).split("|");
> +            for (let idx = vals.length - 1; idx >= 0; idx--) {
> +                if (vals[idx].trim() == "") {
> +                    vals.splice(idx, 1);
> +                    Components.utils.reportError("[calExtract] Faulty extraction pattern " +

You can use new template strings here:

Components.utils.reportError(`[calExtract] Faulty extraction pattern ${value} for ${name}`);
Attachment #8766520 - Flags: review?(philipp) → review+
(In reply to Philipp Kewisch [:Fallen] from comment #10)
> You can use new template strings here:
> Components.utils.reportError(`[calExtract] Faulty extraction pattern
> ${value} for ${name}`);

What is the min gecko version required for this feature? 
Just asking in case we have to port this back to current release.
Version: unspecified → Lightning 4.0.8
(In reply to Philipp Kewisch [:Fallen] from comment #10)
> You can use new template strings here:
> 
> Components.utils.reportError(`[calExtract] Faulty extraction pattern
> ${value} for ${name}`);
Is it worth changing just these two? There are many more LOG calls with concatenation all over the file. To me it makes sense to switch all or none.
(In reply to Merike (:merike) from comment #13)
> Is it worth changing just these two? There are many more LOG calls with
> concatenation all over the file. To me it makes sense to switch all or none.

I'm fine keeping the old format for now.
Keywords: checkin-needed
Duplicate of this bug: 1239443
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 5.2
You need to log in before you can comment on or make changes to this bug.