Unreachable expression after return statement in toLocalTimeISOString.

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: arai, Assigned: arai, Mentored)

Tracking

unspecified
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 unaffected, firefox39 fixed, firefox40 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

4 years ago
in toLocalTimeISOString in toolkit/components/telemetry/TelemetrySession.jsm, semicolon is placed middle of expression, and it results in milliseconds and timezone are omitted from the return value.

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetrySession.jsm#242
>   return    padNumber(date.getFullYear(), 4)
>     + "-" + padNumber(date.getMonth() + 1, 2)
>     + "-" + padNumber(date.getDate(), 2)
>     + "T" + padNumber(date.getHours(), 2)
>     + ":" + padNumber(date.getMinutes(), 2)
>     + ":" + padNumber(date.getSeconds(), 2);      <= here!
>     + "." + date.getMilliseconds()
>     + sign(tzOffset) + Math.abs(Math.floor(tzOffset / 60))
>     + ":" + Math.abs(tzOffset % 60);

not sure which is the right solution to remove the semicolon or remove remaining expressions.
Assignee

Comment 1

4 years ago
what is the expected behavior here?
Flags: needinfo?(gfritzsche)
Ah, thanks, good catch.

The right solution would be to remove the semicolon, ideally the xpcshell tests in toolkit/components/telemetry/tests/unit continue to run through fine.

Were you interested in taking this?
Flags: needinfo?(gfritzsche)
Blocks: 1120356
Mentor: gfritzsche
Assignee

Comment 3

4 years ago
thank, I take this :D
Assignee: nobody → arai.unmht

Updated

4 years ago
Component: Client: Desktop → Telemetry
Product: Firefox Health Report → Toolkit
Comment on attachment 8592229 [details] [diff] [review]
Remove unnecessary semicolon in toLocalTimeISOString in TelemetrySession.jsm.

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

Thank you, that looks good.
Attachment #8592229 - Flags: review?(gfritzsche) → review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b9e8a84c7af4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Depends on: 1154717
Depends on: 1154856
Comment on attachment 8592229 [details] [diff] [review]
Remove unnecessary semicolon in toLocalTimeISOString in TelemetrySession.jsm.

Approved for Aurora. For approval request see bug 1139460 comment 42. For approval comments see bug 1139460 comment 43.
Attachment #8592229 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.