Comment 13 for bug 174672

Comment on attachment 204328
Patch v4

This is looking good now. The only things I have left are nits.

First of all, bring back at least the introductory comment, so people can quickly see what you're doing with this block. You might also want to include comments about why 'try' and 'if' are needed here.

+ var url
semicolon please.

+ try {
+ url = srGetStrBundle("chrome://calendar/locale/calendar.properties")
+ .GetStringFromName("holidayCalendarURL");
+ }
+ catch(ex) {}
1.) Standard indenting is 4 spaces. You see to have done more. (same in the if block)
2.) the closing brace ('}') should be aligned with the start of the code that opened it. In other words, it should be directly under the 't' in 'try'
3.) 'catch' goes on the same line as the closing brace of try. (see line 199 of this file)

+# LOCALIZATION NOTE (holidayCalendarURL):
Now that we've gone through all this trouble to handle cases where localizers don't set this, be sure to mention that it's OK for them to leave it blank if there isn't a holiday file for their locale.

Nice work here. I think we'll get some nice user-feedback on this feature.