Last Comment Bug 1080659 - Converting email into task/event fails when localization uses regular expression special characters
: Converting email into task/event fails when localization uses regular express...
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Lightning Only (show other bugs)
: Lightning 3.3
: All All
-- normal (vote)
: 3.9
Assigned To: Merike Sell (:merike)
:
:
Mentors:
: 1090809 1119511 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-09 09:07 PDT by frankaen
Modified: 2015-04-18 08:33 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
1080659 (1.21 KB, patch)
2014-10-28 14:26 PDT, Merike Sell (:merike)
philipp: review+
Details | Diff | Splinter Review
1080659 (3.84 KB, patch)
2014-11-30 08:04 PST, Merike Sell (:merike)
philipp: review+
Details | Diff | Splinter Review

Description User image frankaen 2014-10-09 09:07:02 PDT
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 User image Merike Sell (:merike) 2014-10-09 12:44:26 PDT
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 User image Stefan Sitter 2014-10-09 12:50:49 PDT
Maybe same as Bug 1015748?
Comment 3 User image Gou Zhuang 2014-10-09 20:20:14 PDT
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 User image Stefan Sitter 2014-10-09 21:43:02 PDT
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 User image Gou Zhuang 2014-10-09 23:39:35 PDT
Thanks Stefan. I worked around this issue by removing chrome/calendar-zh-TW.jar from the installed extension.
Comment 6 User image frankaen 2014-10-10 01:10:26 PDT
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 User image frankaen 2014-10-10 01:14:45 PDT
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 User image Merike Sell (:merike) 2014-10-11 04:14:51 PDT
(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 User image frankaen 2014-10-11 10:15:28 PDT
(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 User image Merike Sell (:merike) 2014-10-11 11:39:39 PDT
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 User image Stefan Plewako [:stef] 2014-10-20 06:16:36 PDT
(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 User image Merike Sell (:merike) 2014-10-28 14:26:31 PDT
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 13 User image Chris 2014-10-29 04:24:17 PDT
*** Bug 1090809 has been marked as a duplicate of this bug. ***
Comment 14 User image Chris 2014-10-29 08:05:41 PDT
Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Works for me.
Comment 15 User image Chris 2014-10-29 08:19:31 PDT
@Merike Forgot: thanks :)
Comment 16 User image Philipp Kewisch [:Fallen] 2014-11-06 07:53:49 PST
Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Looks good, r=philipp
Comment 17 User image Merike Sell (:merike) 2014-11-10 07:37:31 PST
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 User image Merike Sell (:merike) 2014-11-30 08:04:16 PST
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 User image frankaen 2014-12-19 04:23:43 PST
(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 User image Richard Marti (:Paenglab) 2014-12-27 10:13:50 PST
https://hg.mozilla.org/comm-central/rev/bb2bca6a8188
Comment 21 User image Merike Sell (:merike) 2015-01-13 12:45:20 PST
*** Bug 1119511 has been marked as a duplicate of this bug. ***
Comment 22 User image frankaen 2015-04-17 10:42:32 PDT
I do not know if the fix has been implemented, but the bug still exists.
Comment 23 User image Martin Schröder [:mschroeder] 2015-04-18 02:09:04 PDT
(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 User image frankaen 2015-04-18 04:46:02 PDT
L 3.3.3
Comment 25 User image Merike Sell (:merike) 2015-04-18 08:33:35 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.