Closed Bug 1080659 Opened 5 years ago Closed 5 years ago

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

Categories

(Calendar :: Lightning Only, defect)

Lightning 3.3
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: frankaen, Assigned: merike)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Flags: needinfo?(frankaen)
Maybe same as Bug 1015748?
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
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.
Thanks Stefan. I worked around this issue by removing chrome/calendar-zh-TW.jar from the installed extension.
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");
Flags: needinfo?(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
(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.
Flags: needinfo?(splewako)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Converting email into task/event does not work in TB 31.1.2 and L 3.3.1 → Converting email into task/event fails when localization uses regular expression special characters
(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? :)
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.
Assignee: nobody → gasell+mozilla
Status: NEW → ASSIGNED
(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.
Flags: needinfo?(splewako) → needinfo?(sprussak)
Attached patch 1080659 (obsolete) — Splinter Review
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.
Attachment #8512998 - Flags: review?(philipp)
Duplicate of this bug: 1090809
Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Works for me.
@Merike Forgot: thanks :)
Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Looks good, r=philipp
Attachment #8512998 - Flags: review?(philipp) → review+
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.
Attached patch 1080659Splinter Review
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
Attachment #8512998 - Attachment is obsolete: true
Attachment #8530643 - Flags: review?(philipp)
Attachment #8530643 - Flags: review?(philipp) → review+
(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.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/bb2bca6a8188
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.9
Duplicate of this bug: 1119511
Flags: needinfo?(sprussak)
I do not know if the fix has been implemented, but the bug still exists.
(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?
Flags: needinfo?(frankaen)
L 3.3.3
Flags: needinfo?(frankaen)
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.
You need to log in before you can comment on or make changes to this bug.