Closed
Bug 1470609
Opened 6 years ago
Closed 6 years ago
calUtilsCompat.jsm's calGetString references undefined cal
Categories
(Calendar :: Internal Components, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
6.2
People
(Reporter: public, Assigned: MakeMyDay)
References
Details
Attachments
(1 file, 1 obsolete file)
5.44 KB,
patch
|
MakeMyDay
:
review+
MakeMyDay
:
approval-calendar-beta+
Fallen
:
approval-calendar-esr+
|
Details | Diff | Splinter Review |
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•6 years ago
|
||
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).
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
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().
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8987367 -
Flags: review?(philipp)
Attachment #8987367 -
Flags: approval-calendar-beta?(philipp)
Comment 3•6 years ago
|
||
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•6 years ago
|
||
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?
Attachment #8987367 -
Flags: review?(philipp)
Attachment #8987367 -
Flags: review+
Attachment #8987367 -
Flags: approval-calendar-beta?(philipp)
Attachment #8987367 -
Flags: approval-calendar-beta+
Comment 5•6 years ago
|
||
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.
Assignee | ||
Comment 6•6 years ago
|
||
Thanks. I made use of global now as proposed.
Attachment #8987367 -
Attachment is obsolete: true
Attachment #8987625 -
Flags: review+
Attachment #8987625 -
Flags: approval-calendar-beta+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 7•6 years ago
|
||
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.
Attachment #8987625 -
Flags: approval-calendar-esr?(philipp)
Assignee | ||
Comment 8•6 years ago
|
||
Yes, this is for both beta branches and for esr60 (but not esr52).
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/9ae99c6bd8b5 Fix leftover from cal.calGetString conversion. r=philipp
Updated•6 years ago
|
Version: Lightning 6.2 → Lightning 6.5
Updated•6 years ago
|
Target Milestone: --- → 6.5
Version: Lightning 6.5 → Lightning 6.2
Comment 10•6 years ago
|
||
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
Target Milestone: 6.5 → 6.2
Updated•6 years ago
|
Attachment #8987625 -
Flags: approval-calendar-esr?(philipp) → approval-calendar-esr+
You need to log in
before you can comment on or make changes to this bug.
Description
•