1414312 - Perfherder uses toLocaleFormat() which has been removed in Firefox 58

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P1)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

for example, go here:
https://treeherder.mozilla.org/perf.html#/alerts?id=10314

select the improvement and there are two options "copy summary" and "unlink from bug"- choose "copy summary" and you will see this in the firefox console:
Error: G.toLocaleFormat is not a function
[601]/</</q.prototype.getTextualSummary@https://treeherder.mozilla.org/perf.765571126be0e2904a57.bundle.js:48:67194
[577]/</</A.copyTextToClipboard@https://treeherder.mozilla.org/perf.765571126be0e2904a57.bundle.js:48:7278
fn@https://treeherder.mozilla.org/vendor.51f52799a614eccb6b35.bundle.js line 41 > Function:4:362
Qc@https://treeherder.mozilla.org/vendor.51f52799a614eccb6b35.bundle.js:41:436577

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

oddly the only index I see via github search is:
https://github.com/mozilla/treeherder/blob/7233dfd46f63c8d478c1ddbdebe608023d3d32e5/ui/js/models/perf/alerts.js#L96

but github search was last updated october 3rd, possibly this is out of date.

Comment 2

[Ed Morley [:emorley]]

The toLocaleFormat() call has existed in Perfherder unchanged since it landed in bug 1285368 (July 2016), and there is nothing else of note in the git log of that file recently - so this doesn't appear to be a regression on our side.

The MDN page for it:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleFormat

...led me to bug 818634 which just landed, and has removed toLocaleFormat() in Nightly (Firefox 58).

I'm presuming Perfherder should be using something other than this function to format the dates.

Rob, I don't suppose you'd mind taking a look? :-)

Comment 3

[Ed Morley [:emorley]]

Oh it appears toLocaleFormat() was Firefox only - so I guess the fix for this will also make this feature work for the first time in other browsers too :-)

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

if your goal is for the user to read and understand the date, use Intl.DateTimeFormat (or date.toLocaleString which is the same). If you need a parsable/computer-readable date, format ISO date with `{$day}-${month}-${year}` etc.

Comment 5

[GitHub Autolander Bot]

Attachment: [treeherder] wlach:1414312 > mozilla:master

Comment 6

[William Lachance (:wlach)]

Comment on attachment 8925078 [details] [review]
[treeherder] wlach:1414312 > mozilla:master

Sorry, I missed the conversation here while fixing the problem. :)

I think the short-term fix is to just use toUTCString which is good enough for our purposes and seems to be standard. The purpose of the textual summary is to provide a standard template inside bugzilla reports that is readable by audience who understands english and is aware of utc. More advanced localization stuff is not required.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8925078 [details] [review]
[treeherder] wlach:1414312 > mozilla:master

thanks for the fix

Comment 8

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/117bfd620142e34f12093eb3ae81bff483401224
Bug 1414312 - Fix perfherder textual alert summary copy function on recent Firefoxen (#2910)

It used toLocaleFormat, which was never standard and seems to no longer be defined
in recent Firefox versions. Use toUTCString instead, which seems to be available
everywhere.

Comment 9

[William Lachance (:wlach)]

I think we're ok here now.