Make calUtils.js functions independent of window

RESOLVED FIXED in 5.7

Status

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

unspecified
Dependency tree / graph

Details

Attachments

(2 attachments, 1 obsolete attachment)

Some of the calUtils.js functions are meant to run in a scope that has a window. Moving to calUtils.jsm we should stop doing this.
Posted patch Fix - v1 (obsolete) — Splinter Review
Attachment #8846663 - Flags: review?(makemyday)
Blocks: 1347149

Comment 2

2 years ago
Comment on attachment 8846663 [details] [diff] [review]
Fix - v1

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

Thanks. r+ with the missing prefixes added.

::: calendar/base/content/dialogs/calendar-migration-dialog.js
@@ +559,3 @@
>                      break;
>                  default:
> +                    cal.showError(calGetString("calendar", "unableToRead") + icsFile.path + "\n"+ ex, window);

Please use cal.calGetString() instead (here an at some other places in this patch).
Attachment #8846663 - Flags: review?(makemyday) → review+
I have the cal.calGetString changes lined up in a different patch in my queue, I'd prefer to fix only the issue in the bug :) Will land this when I have my queue cleaned up.
Priority: -- → P1
Posted patch Fix v2Splinter Review
After talking to Philipp I updated the patch, editing an additional usage of showError() in calendar-alarm-dialog.js. Carrying over r+.
Attachment #8846663 - Attachment is obsolete: true
Attachment #8871837 - Flags: review+
Thanks for updating the patch! The next one in my queue is bug 1347149.
Keywords: checkin-needed

Comment 6

2 years ago
https://hg.mozilla.org/comm-central/rev/a4458b9e5377a6cb415bedee8fddf7e677a2b7ce
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 5.7
I am getting:

> Timestamp: 5/29/2017, 1:44:05 PM
> Error: ReferenceError: gCalendarStatusFeedback is not defined
> Source File: resource://calendar/modules/calUtils.jsm -> file:///C:/Program%20Files/SeaMonkey
> /extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calUtils.js
> Line: 1871

Shouldn't 
 comp.setStatusObserver(gCalendarStatusFeedback, chromeWindow);
be 
 comp.setStatusObserver(aWindow.gCalendarStatusFeedback, chromeWindow);
Flags: needinfo?(mschroeder)
Posted patch Follow-up fixSplinter Review
Flags: needinfo?(mschroeder)
Attachment #8872355 - Flags: review?(ssitter)

Comment 9

2 years ago
Comment on attachment 8872355 [details] [diff] [review]
Follow-up fix

That looks right.
Attachment #8872355 - Flags: review?(ssitter) → review+
You need to log in before you can comment on or make changes to this bug.