490243 - reference to undefined property this.mExceptionMap[getRidKey(aRecurrenceId)], calRecurrenceInfo.js Line: 744

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b5pre) Gecko/20090425 Shiretoko/3.5b5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b5pre) Gecko/20090426 Calendar/1.0pre

With pref javascript.options.strict true,
get many warnings:
  reference to undefined property this.mExceptionMap[getRidKey(aRecurrenceId)]
  calendar-js/calRecurrenceInfo.js Line: 744


Reproducible: Always

Steps to Reproduce:
1. Create new profile.
2. Set pref javascript.options.strict true
(set via Tools > Options > Advanced > General > Config Editor
 search for "strict", double click it to toggle)
3. create 1 new event (in default home calendar) via event dialog
   that repeats weekly
4. restart
5. open error console, view all

Actual Results:  
the warning
  reference to undefined property this.mExceptionMap[getRidKey(aRecurrenceId)]
  calendar-js/calRecurrenceInfo.js Line: 744	     
occurs 11 times (for 1 event)


Expected Results:  
the warning does not appear.


can slow down startup

workaround is to turn off the pref.

Comment 1

[gekacheka]

Attachment: v1 patch: make result explict in undefined case

(patch -l -p 1 -i file.patch)

Warning occurs at call stack
calRecurrenceInfo.getExceptionFor
calRecurrenceInfo.getOccurrenceFor
calRecurrenceInfo.getOccurrences
calMemoryCalendar.getItems

A simple fix is to return an explicit value in the case where it
undefined, such as by returning
this.mExceptionMap[getRidKey(aRecurrenceId)] || undefined
This is ok in this case since the map value should never be a
javascript false value such as false, null, or "".

Comment 2

[Daniel Boelzle [:dbo]]

Comment on attachment 374698 [details] [diff] [review]
v1 patch: make result explict in undefined case

>     getExceptionFor: function cRI_getExceptionFor(aRecurrenceId) {
>         this.ensureBaseItem();
>-        return this.mExceptionMap[getRidKey(aRecurrenceId)];
>+        // to avoid strict "reference to undefined property" warning, appending
>+        // "|| undefined" gives explicit result in case where property undefined
>+        // (or false, or null, or "", but here it should never be those values)
>+        return this.mExceptionMap[getRidKey(aRecurrenceId)] || undefined;

You can safely return |null| instead of |undefined| in case there's no exception record.

r=dbo

Comment 3

[Martin Schröder [:mschroeder]]

gekacheka, is your patch ready for checkin?

Comment 4

[Stefan Sitter [:ssitter]]

Pushed to comm-central <https://hg.mozilla.org/comm-central/rev/1076fdb4f9e9>

--> FIXED

Comment 5

[Daniel Boelzle [:dbo]]

(In reply to comment #2)
> >+        return this.mExceptionMap[getRidKey(aRecurrenceId)] || undefined;
> 
> You can safely return |null| instead of |undefined| in case there's no
> exception record.
I'd really appreciate this and change the comment, too. calIRecurrenceInfo::getExceptionFor() is defined to return null in case an exceptional record does not exist.

Comment 6

[Stefan Sitter [:ssitter]]

Reopen to address comment #5

Comment 7

[gekacheka]

Attachment: patch 2 v1: return null if undefined

(patch -l -p 1 -i file.patch)

As requested, return null rather than undefined if not found.
Documents that it implements interface that specifies null be returned.

Comment 8

[Daniel Boelzle [:dbo]]

Comment on attachment 377932 [details] [diff] [review]
patch 2 v1: return null if undefined

r=dbo

Comment 9

[Daniel Boelzle [:dbo]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/2923391a4a2c>

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".