1280824 - Thunderbird freezes on Convert / Event

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Matěj Cepl]

Attachment: 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.

Comment 1

[Matěj Cepl]

Attachment: test message

Comment 2

[Merike Sell (:merike)]

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.

Comment 3

[Matěj Cepl]

(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).

Comment 4

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

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.

Comment 5

[Stefan Sitter [:ssitter]]

Does the issue still exists using the latest release (Thunderbird 45.x with Lightning 4.7.x)?

Comment 6

[Matěj Cepl]

(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?

Comment 7

[Merike Sell (:merike)]

(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.

Comment 8

[Merike Sell (:merike)]

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.

Comment 9

[Merike Sell (:merike)]

Attachment: emptyPattern

This should make extraction ignore empty variants causing freezes. Error is still logged.

Comment 10

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

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}`);

Comment 11

[Stefan Sitter [:ssitter]]

(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.

Comment 12

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

As per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals they have been available since Gecko 34.

Comment 13

[Merike Sell (:merike)]

(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.

Comment 14

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

(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.

Comment 16

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/aba5713fd0e70ee026d35608a93562c70f22cd7c
Bug 1280824 - Thunderbird freezes on Convert / Event. r=philipp