Last Comment Bug 1176936 - Event extraction broken with single locale Lightning
: Event extraction broken with single locale Lightning
Status: RESOLVED FIXED
: regression
Product: Calendar
Classification: Client Software
Component: Lightning Only (show other bugs)
: Lightning 4.0.0.1
: Unspecified Unspecified
-- normal (vote)
: 4.0.1.2
Assigned To: Merike Sell (:merike)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-23 13:54 PDT by Merike Sell (:merike)
Modified: 2015-08-07 03:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
bug1176936 (5.19 KB, patch)
2015-07-14 02:07 PDT, Merike Sell (:merike)
philipp: review+
Details | Diff | Splinter Review

Description User image Merike Sell (:merike) 2015-06-23 13:54:50 PDT
This is per https://bugzilla.mozilla.org/show_bug.cgi?id=1049443#c38

It appears that when I let Thunderbird on linux download single locale Lightning the packaging is different than in multi-locale build. Retrieving patterns does not work and guessing dates and times fails silently. At least on linux when I download multi-locale from AMO then it works again.

It used to be that packaging was similar for single locale and multi-locale builds. This way you could still use patterns for the one language included and it wasn't necessary to check in code which build was used. I'm unsure if single locale directory structure is only different from multi-locale build now or also different on different platforms?
Comment 1 User image Philipp Kewisch [:Fallen] 2015-06-24 00:22:26 PDT
Ah yes, there are some differences in packaging due to the way the python packager works. IIRC then the locale files are now all in the toplevel chrome.jar of the extension, but just for the packaged builds.
Comment 2 User image Merike Sell (:merike) 2015-06-25 12:23:01 PDT
After looking at it for some time it does not seem too easy to fix.

As far as I can tell there isn't an easy way to map chrome.jar in python packaged version to resource://calendar/chrome address or I didn't come up with the right manifest instruction. I could detect how many locales are available and use regular chrome://calendar/locale/calendar-extract.properties when only one is available but this has following potential downsides:
* if there ever is a packaged build including multiple locales we'd use only default
* in places where the calExtract.jsm uses average charcode and dictionaries it assumes it knows which locale we're using, with a generic url these parts need a rewrite to work properly; I'd rather avoid that if possible
Basically, having an url for both AMO and packaged build which includes language code would be best.

Is it the case that we expect most users to be on a packaged build with one locale now? If so, how hard would it be to include patterns for more languages so that multilanguage extraction isn't lost? Is packaged build structure intentionally different or can it be changed? Any other thoughts on what might be the best way forward here?
Comment 3 User image Philipp Kewisch [:Fallen] 2015-06-29 07:06:24 PDT
Unfortunately we can't change the packaging structure, it is done by the python packager and common to all toolkit apps.

I believe you can use an url like this (untested)

jar:resource://calendar/chrome.jar!/calendar-LOCALE/locale/LOCALE/calendar/calendar-extract.properties

Can you try reading from one uri first, and if that fails then read from the next uri?

Including multiple Lightning locales in each Thunderbird package is near impossible with the current way things are set up. The packager doesn't have all locales available when running the repacks, and the build process will also not update the en-US builds. The only way I see to restore it would be to add some button that triggers installation from AMO, but that is not ideal either. I could modify the version strings when uploading to AMO, but that would give us silly things like 4.0.0.1.1 for Thunderbird 38.0.1 and 4.0.1.1 for Thunderbird 38.1.0.
Comment 4 User image Merike Sell (:merike) 2015-07-08 12:53:34 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #3)
> I believe you can use an url like this (untested)
> 
> jar:resource://calendar/chrome.jar!/calendar-LOCALE/locale/LOCALE/calendar/
> calendar-extract.properties
Great, this one works. I believe I tried it the other way around like resource:jar:// and then it didn't.

> Can you try reading from one uri first, and if that fails then read from the
> next uri?

With above working I can alternate between that and old url based on the number of locales included in Lightning. That itself isn't a problem. I'll try to have a patch ready for that in the next few days.
 
> Including multiple Lightning locales in each Thunderbird package is near
> impossible with the current way things are set up. The packager doesn't have
> all locales available when running the repacks, and the build process will
> also not update the en-US builds.
I wasn't actually thinking about full locale but rather patterns only. With other locales unavailable that is impossible then.

> The only way I see to restore it would be
> to add some button that triggers installation from AMO, but that is not
> ideal either. I could modify the version strings when uploading to AMO, but
> that would give us silly things like 4.0.0.1.1 for Thunderbird 38.0.1 and
> 4.0.1.1 for Thunderbird 38.1.0.
What happens when someone installs from AMO and then they update Thunderbird? Do they go from multilocale build to single locale one? Someone wanting multiple locales detection would then need to overwrite with AMO build after each update, or at least for now

I suppose it would be possible to extract just patterns from AMO build with a script and upload them for download similarly to holiday calendars but this would need two pieces, one doing the extraction and the other downloading and using them from Lightning. Doesn't seem easy either. Anyway this is outside the scope of this bug. Lets first make sure that single locale build can use patterns again.
Comment 5 User image Philipp Kewisch [:Fallen] 2015-07-08 15:23:33 PDT
(In reply to Merike (:merike) from comment #4)
> > Can you try reading from one uri first, and if that fails then read from the
> > next uri?
> 
> With above working I can alternate between that and old url based on the
> number of locales included in Lightning. That itself isn't a problem. I'll
> try to have a patch ready for that in the next few days.

I think reading both urls would be cleaner, but if its easier to check for locale count I'm fine with it.

>  
> > Including multiple Lightning locales in each Thunderbird package is near
> > impossible with the current way things are set up. The packager doesn't have
> > all locales available when running the repacks, and the build process will
> > also not update the en-US builds.
> I wasn't actually thinking about full locale but rather patterns only. With
> other locales unavailable that is impossible then.
Yeah, not all repos are checked out when the repacks happen :-/


> 
> > The only way I see to restore it would be
> > to add some button that triggers installation from AMO, but that is not
> > ideal either. I could modify the version strings when uploading to AMO, but
> > that would give us silly things like 4.0.0.1.1 for Thunderbird 38.0.1 and
> > 4.0.1.1 for Thunderbird 38.1.0.
> What happens when someone installs from AMO and then they update
> Thunderbird? Do they go from multilocale build to single locale one? Someone
> wanting multiple locales detection would then need to overwrite with AMO
> build after each update, or at least for now
Yes, I fear that they will fall back to the single-locale package again.

> 
> I suppose it would be possible to extract just patterns from AMO build with
> a script and upload them for download similarly to holiday calendars but
> this would need two pieces, one doing the extraction and the other
> downloading and using them from Lightning. Doesn't seem easy either. Anyway
> this is outside the scope of this bug. Lets first make sure that single
> locale build can use patterns again.
It could be an extension like the timezones extension, which we can have updated on AMO. I agree we should figure this out in another bug though.
Comment 6 User image Merike Sell (:merike) 2015-07-14 02:07:16 PDT
Created attachment 8633369 [details] [diff] [review]
bug1176936

This should fix packaged build. Warning will be wrong for local builds as these are structured like AMO build but contain only en-US. Should be correct for any single locale packaged build though.
Comment 7 User image Philipp Kewisch [:Fallen] 2015-07-20 03:14:11 PDT
Comment on attachment 8633369 [details] [diff] [review]
bug1176936

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

r=philipp

I'll see if I can include this in 4.0.1.1 since I haven't yet managed to release it.
Comment 8 User image Philipp Kewisch [:Fallen] 2015-07-22 12:00:46 PDT
Backported to releases/comm-aurora changeset 39e76d5e0716
Comment 9 User image Philipp Kewisch [:Fallen] 2015-07-22 12:01:59 PDT
Backported to releases/comm-beta changeset 5747795bcfbd
Comment 10 User image Philipp Kewisch [:Fallen] 2015-07-22 12:04:01 PDT
Backported to releases/comm-esr38 changeset 696bec67443a
Comment 11 User image Philipp Kewisch [:Fallen] 2015-07-22 12:39:51 PDT
Leaving checkin-needed for comm-central

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