1080659 - Converting email into task/event fails when localization uses regular expression special characters

Status: RESOLVED FIXED

Description

[frankaen]

User Agent: Opera/9.80 (Windows NT 6.2; WOW64) Presto/2.12.388 Version/12.16

Steps to reproduce:

Upgrade TB to 31.1.2 and automatically L to 3.3.1.
TB, mark an email,
RMB on it,
click the option to convert it into task or event.



Actual results:

Clicking the option to convert an email into task or event does not work, no task appears in Lightning.


Expected results:

The same what before the upgrade. The task should have appeared in Lightning.

Comment 1

[Merike Sell (:merike)]

Do you see any errors in error console? Are you using any other extensions besides Lightning? Converting to tasks and events works for me on linux using the same Thunderbird and Lightning versions.

Comment 2

[Stefan Sitter [:ssitter]]

Maybe same as Bug 1015748?

Comment 3

[Gou Zhuang]

I have the same issue. The behavior depends on the email to convert, for some it works, for some it doesn't.

When it fails, the following were shown in error console:
Timestamp: 10/10/14, 10:43:49
Error: [calExtract] Faulty extraction pattern 從 %1$S 時 | 從 %1$S 點 | %1$S - | %1$S ~ | %1$S 點到 | %1$S 時到 | %1$S 點至 | %1$S 時至 | for from.hour
Source File: resource://calendar/modules/calExtract.jsm
Line: 1114

Timestamp: 10/10/14, 10:43:49
Error: SyntaxError: invalid range in character class
Source File: resource://calendar/modules/calExtract.jsm
Line: 1183

Comment 4

[Stefan Sitter [:ssitter]]

Gou Zhuang, this is a known problem caused by an error in Chinese localization (Bug 1049443). Workaround: upgrade to Lightning 3.3.1 that doesn't contain Chinese localization anymore.

Comment 5

[Gou Zhuang]

Thanks Stefan. I worked around this issue by removing chrome/calendar-zh-TW.jar from the installed extension.

Comment 6

[frankaen]

Thanks Gou Zhuang, does not work for me, I just do not have this file in my installation, so this does not trigger the bug.

Error console
Timestamp: 2014-10-10 10:06:00
Error: SyntaxError: invalid quantifier
Source File: resource://calendar/modules/calExtract.jsm
Line: 1232

And this is this line:
        re = new RegExp("^" + ch + "(" + this.getPatterns("no.datetime.suffix") + ")", "ig");

Comment 7

[frankaen]

Merike, yes, error above,
the other add-ons:
Classic TB2 Options 1.3.0
Copy Folder 1.61
Folderpane Tools 0.6.1
keyconfig 20110522
Profile Switcher 1.6.2

Comment 8

[Merike Sell (:merike)]

(In reply to frankaen from comment #6)
> Error: SyntaxError: invalid quantifier
> Source File: resource://calendar/modules/calExtract.jsm
> Line: 1232
> 
> And this is this line:
>         re = new RegExp("^" + ch + "(" +
> this.getPatterns("no.datetime.suffix") + ")", "ig");

Let me guess based on your email address and http://mxr.mozilla.org/l10n-mozilla-release/source/pl/calendar/chrome/calendar/calendar-extract.properties#12 that you're using Thunderbird in Polish?

The problem with invalid quantifier is that a + is used in a pattern directly unescaped. I think it would make sense to add escaping to Lightning so that localizers wouldn't need to know about special characters in regular expressions. It would be interesting to know the motivation behind adding + as a suffix though, maybe English should also have it... Stefan, could you clarify why you chose to add this one?

Also, for anyone experiencing errors that affect a specific locale, as a workaround you can add conversion button to message header toolbar and from there choose another language for converting. It also works the other way around, if you can convert using another language then there is a locale specific issue.

Comment 9

[frankaen]

(In reply to Merike (:merike) from comment #8)

Merike, yes, Polish.

OK, I did what you said, and works converting into English GB or UK. Hence, locale issue.
To whom should I direct my humble request then? :)

Comment 10

[Merike Sell (:merike)]

From user perspective it can be fixed both by Polish localizer (already CCed here) and also in Lightning itself. I marked this bug as new because it should be fixed in Lightning irrespective of whether the pattern causing trouble is changed in Polish or not.

Comment 11

[Stefan Plewako [:stef]]

(In reply to Merike (:merike) from comment #8)
> The problem with invalid quantifier is that a + is used in a pattern
> directly unescaped. I think it would make sense to add escaping to Lightning
> so that localizers wouldn't need to know about special characters in regular
> expressions. It would be interesting to know the motivation behind adding +
> as a suffix though, maybe English should also have it... Stefan, could you
> clarify why you chose to add this one?

Maybe it was something with timezones but honestly, I have hard time trying to remember why…
We should add l10n note or escaping for the strings even if Sara (current lt translator) will decide to remove that sign.

Comment 12

[Merike Sell (:merike)]

Created attachment 8512998 [details] [diff] [review]
1080659

This ought to fix it for Polish and also Korean which makes use of ( and even \ characters :)
Regular expression stolen from https://github.com/mozilla/mozmill/commit/ae8d7b65e64d68ba97e5f50ccf2093bc084edfeb and | removed because that should be treated specially on purpose. I also noticed a small logging error while fixing this.

Comment 14

[Chris]

Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Works for me.

Comment 15

[Chris]

@Merike Forgot: thanks :)

Comment 16

[Philipp Kewisch [:Fallen] [:📆]]

Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Looks good, r=philipp

Comment 17

[Merike Sell (:merike)]

Right, so it still looks good and fixes the initial problem but I finally had a chance to run tests locally and it does not pass. Need to look into it, not ready for checking in.

Comment 18

[Merike Sell (:merike)]

Created attachment 8530643 [details] [diff] [review]
1080659

This time it passes tests too :)

Changes from previous patch:
* * needs to be escaped before spaces are set to match any amount of whitespace, else it gets escaped when it should not
* $ needs special casing because of localization variables
* adding a test to make sure dollar sign works in patterns
* while testing this I found an error in extracting hours which can result in afternoon hours extracted as morning ones

Comment 19

[frankaen]

(In reply to Merike (:merike) from comment #18)
> Created attachment 8530643 [details] [diff] [review]
> 1080659
> 
> This time it passes tests too :)
> 
> Changes from previous patch:
[...]
> * while testing this I found an error in extracting hours which can result
> in afternoon hours extracted as morning ones
===
FYI, I have found the info about this as a bug, and a problem. Sorry for the offtopic, and the lack of details.

Comment 20

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/bb2bca6a8188

Comment 22

[frankaen]

I do not know if the fix has been implemented, but the bug still exists.

Comment 23

[Martin Schröder [:martinschroeder]]

(In reply to frankaen from comment #22)
> I do not know if the fix has been implemented, but the bug still exists.

What version of Lightning are you using when experiencing the problem?

Comment 24

[frankaen]

L 3.3.3

Comment 25

[Merike Sell (:merike)]

Target milestone above is set to 3.9 meaning the fix is available in versions 3.9 and later. That's why you still experience it with 3.3 series.