Tooru Fujisawa [:arai] Assignee
	Description • 9 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.

	Tooru Fujisawa [:arai] Assignee
	Comment 1 • 9 years ago

what is the expected behavior here?

	Georg Fritzsche [:gfritzsche]
	Comment 2 • 9 years ago

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?

	Tooru Fujisawa [:arai] Assignee
	Comment 3 • 9 years ago

thank, I take this :D

	Tooru Fujisawa [:arai] Assignee
	Comment 4 • 9 years ago

Just removed semicolon.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=57403fdcae3a

	Georg Fritzsche [:gfritzsche]
	Comment 5 • 9 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9e8a84c7af4

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/b9e8a84c7af4

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 8 • 9 years ago

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.

	Georg Fritzsche [:gfritzsche]
	Comment 9 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/9c3834da969e