Closed Bug 1115965 Opened 5 years ago Closed 5 years ago

Provide filename and line number in cal.WARN and cal.ERROR

Categories

(Calendar :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: darktrojan, Assigned: darktrojan)

Details

Attachments

(1 file, 1 obsolete file)

nsIScriptError allows us to show the filename and line number of the error, so we should use it.
Attached patch 1115965-1.diff (obsolete) — Splinter Review
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #8541909 - Flags: review?(philipp)
Comment on attachment 8541909 [details] [diff] [review]
1115965-1.diff

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

r+ with these changes:

::: calendar/base/src/calUtils.js
@@ +925,3 @@
>                                  .createInstance(Components.interfaces.nsIScriptError);
> +    scriptError.init(aMessage, frame.filename.split(" -> ").pop(),
> +                     null, frame.lineNumber, 0,

Is it possible that frame.filename is null? I know its the case when calling from C code, although this doesn't apply here. But maybe there are other cases?

Also, might was well also pass columnNumber.


same applies for cal.ERROR of course.
Attachment #8541909 - Flags: review?(philipp) → review+
Attached patch 1115965-2.diffSplinter Review
I suppose frame.filename could be null, although I think it's unlikely. Best to be safe, I suppose.
Attachment #8541909 - Attachment is obsolete: true
Attachment #8541911 - Flags: review?(philipp)
Comment on attachment 8541911 [details] [diff] [review]
1115965-2.diff

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

Thanks for the patch, r=philipp
Attachment #8541911 - Flags: review?(philipp) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/2e6ca0b22e1e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 3.9
You need to log in before you can comment on or make changes to this bug.