1470609 - calUtilsCompat.jsm's calGetString references undefined cal

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[rsjtdrjgfuzkfg (Dirk Steinmetz)]

STR (one of many):
0. Thunderbird 60b8, bundled Lightning 6.2
1. Get invited for an event
2. Accept the invitation
3. Double-click on the resulting entry

Expected:
Some status information is shown (en-US: "You have accepted this invitation") between toolbar and the "General" section, no fatal messages in the error console

Actual:
No status information shown, error console lists a deprecation warning (intended), then
> ReferenceError: cal is not defined
in calUtilsCompat.jsm:193

Reason / Patch:
calUtilsCompat.jsm:193 is
> return cal.l10n.getAnyString(aComponent, aBundleName, aStringName, aParams);
but should be (imho)
> return global.l10n.getAnyString(aComponent, aBundleName, aStringName, aParams);

Comment 1

[Martin Schröder [:mschroeder]]

This sounds reasonable, and the culprit is a missed migration from "cal.getString()" in calendar-summary-dialog.js (and it does not seem to be the only one).

Comment 2

[[:MakeMyDay]]

Attachment: FixLeftoverCalGetStringConversion-V1.diff

Thanks for the report. This patch makes sure the cal.10n call from within calUtilsCompat works and converts the remainders of cal.calGetString() to cal.l10n.getString().

Comment 3

[Martin Schröder [:mschroeder]]

Comment on attachment 8987367 [details] [diff] [review]
FixLeftoverCalGetStringConversion-V1.diff

Is it a good idea to import calUtils.jsm in calUtilsCompat.jsm? Using the variable "global" as suggested by Dirk seems to be a valid solution.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8987367 [details] [diff] [review]
FixLeftoverCalGetStringConversion-V1.diff

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

r+ with this comment considered:

::: calendar/base/modules/calUtilsCompat.jsm
@@ +4,5 @@
>  
>  /*
>   * Backwards compat for calUtils migration.
>   */
> +ChromeUtils.import("resource://calendar/modules/calUtils.jsm");

I'm not sure this is a good idea, as calUtils.jsm also includes calUtilsCompat.jsm. Maybe in this special case we should import calL10NUtils.jsm directly, along with a comment as to why?

Comment 5

[Philipp Kewisch [:Fallen][📱📆]]

Ah I see Martin already commented on that, thats what I get for going through email without reading the actual bug :) Both using global and a direct import would work for me.

Comment 6

[[:MakeMyDay]]

Attachment: FixLeftoverCalGetStringConversion-V2.diff

Thanks. I made use of global now as proposed.

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8987625 [details] [diff] [review]
FixLeftoverCalGetStringConversion-V2.diff

Which beta do you mean here? 60 and/or 62? I assume 60+62.

Also, I need 6.5 as a target milestone.

Comment 8

[[:MakeMyDay]]

Yes, this is for both beta branches and for esr60 (but not esr52).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9ae99c6bd8b5
Fix leftover from cal.calGetString conversion. r=philipp

Comment 10

[Jorg K (CEST = GMT+2)]

TB 60 beta 10, Calendar 6.2 (BETA_60_CONTINUATION branch):
https://hg.mozilla.org/releases/comm-beta/rev/118b0dfa909ecb4eb25cf5156a857a4a3d00dfd1

Beta (TB 62, Calendar 6.4):
https://hg.mozilla.org/releases/comm-beta/rev/c5fd7206a84d04a43e0ed60055eacdd9b45669da

Comment 11

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-esr60/rev/efb1302a8c6c