Converting email into task/event fails when localization uses regular expression special characters
RESOLVED
FIXED
in 3.9
Status
People
(Reporter: frankaen, Assigned: merike)
Tracking
Details
Attachments
(1 attachment, 1 obsolete attachment)
3.84 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
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•4 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•4 years ago
|
||
Maybe same as Bug 1015748?
Comment 3•4 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•4 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•4 years ago
|
||
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
(Assignee) | ||
Comment 8•4 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•4 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
(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•4 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•4 years ago
|
Assignee: nobody → gasell+mozilla
Status: NEW → ASSIGNED
Comment 11•4 years ago
|
||
(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•4 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)
Comment 14•4 years ago
|
||
Comment on attachment 8512998 [details] [diff] [review] 1080659 Review of attachment 8512998 [details] [diff] [review]: ----------------------------------------------------------------- Works for me.
Comment 15•4 years ago
|
||
@Merike Forgot: thanks :)
Comment 16•4 years ago
|
||
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•4 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•4 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)
Updated•4 years ago
|
Attachment #8530643 -
Flags: review?(philipp) → review+
(Reporter) | ||
Comment 19•4 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•4 years ago
|
Keywords: checkin-needed
Comment 20•4 years ago
|
||
https://hg.mozilla.org/comm-central/rev/bb2bca6a8188
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.9
Updated•4 years ago
|
Keywords: checkin-needed
Updated•4 years ago
|
Flags: needinfo?(sprussak)
(Reporter) | ||
Comment 22•4 years ago
|
||
I do not know if the fix has been implemented, but the bug still exists.
Comment 23•4 years ago
|
||
(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)
(Assignee) | ||
Comment 25•4 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.
Description
•