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

RESOLVED FIXED in 3.9

Status

Calendar
Lightning Only
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: frankaen, Assigned: merike)

Tracking

Lightning 3.3

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
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)

Comment 2

3 years ago
Maybe same as Bug 1015748?

Comment 3

3 years ago
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

3 years ago
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

3 years ago
Thanks Stefan. I worked around this issue by removing chrome/calendar-zh-TW.jar from the installed extension.
(Reporter)

Comment 6

3 years ago
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)
(Reporter)

Comment 7

3 years ago
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
(Assignee)

Comment 8

3 years ago
(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)
(Assignee)

Updated

3 years ago
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
(Reporter)

Comment 9

3 years ago
(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? :)
(Assignee)

Comment 10

3 years ago
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)

Updated

3 years ago
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)
(Assignee)

Comment 12

3 years ago
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.
Attachment #8512998 - Flags: review?(philipp)

Updated

3 years ago
Duplicate of this bug: 1090809

Comment 14

3 years ago
Comment on attachment 8512998 [details] [diff] [review]
1080659

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

Works for me.

Comment 15

3 years ago
@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+
(Assignee)

Comment 17

3 years ago
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.
(Assignee)

Comment 18

2 years ago
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
Attachment #8512998 - Attachment is obsolete: true
Attachment #8530643 - Flags: review?(philipp)
Attachment #8530643 - Flags: review?(philipp) → review+
(Reporter)

Comment 19

2 years ago
(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.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/bb2bca6a8188
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.9
Keywords: checkin-needed
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1119511
Flags: needinfo?(sprussak)
(Reporter)

Comment 22

2 years ago
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)
(Reporter)

Comment 24

2 years ago
L 3.3.3
Flags: needinfo?(frankaen)
(Assignee)

Comment 25

2 years ago
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.