Closed Bug 1154228 Opened 5 years ago Closed 5 years ago

Unreachable expression after return statement in toLocalTimeISOString.

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox38 --- unaffected
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: arai, Assigned: arai, Mentored)

References

Details

Attachments

(1 file)

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.
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
thank, I take this :D
Assignee: nobody → arai.unmht
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: 5 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.