1299900 - Warn when Date.prototype.toLocaleFormat is used

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[André Bargull [:anba]]

Prerequisite for bug 818634

Comment 1

[Kohei Yoshino [:kohei]]

The site compat doc can be found at https://www.fxsitecompat.com/en-CA/docs/2015/date-prototype-tolocaleformat-will-be-removed/

Comment 2

[André Bargull [:anba]]

Attachment: bug1299900-part1.patch

Collect telemetry for Date.prototype.toLocaleFormat if a suitable replacement is available (i.e. if Intl isn't disabled).

Comment 3

[André Bargull [:anba]]

Attachment: bug1299900-part2.patch

Warn when Date.prototype.toLocaleFormat is used (and Intl is available).

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8808272 [details] [diff] [review]
bug1299900-part1.patch

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

::: js/src/jsdate.cpp
@@ +2805,5 @@
>      Rooted<DateObject*> dateObj(cx, &args.thisv().toObject().as<DateObject>());
>  
> +#if EXPOSE_INTL_API
> +    if (JSScript *script = cx->currentScript()) {
> +        const char *filename = script->filename();

Nit: * goes with the type, twice.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8808274 [details] [diff] [review]
bug1299900-part2.patch

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

Thanks!

::: js/src/jsdate.cpp
@@ +2814,5 @@
> +        if (!JS_ReportErrorFlagsAndNumberASCII(cx, JSREPORT_WARNING, GetErrorMessage, nullptr,
> +                                               JSMSG_DEPRECATED_TOLOCALEFORMAT))
> +        {
> +            return false;
> +        }

Could use JS_ReportWarning here, but this gets the job done.

Comment 6

[André Bargull [:anba]]

Attachment: bug1299900.patch

Updated to drop part 1 because of bug 1331909. Carrying r+ from jandem.

Comment 7

[André Bargull [:anba]]

Attachment: bug1299900.patch

Updated to apply cleanly on inbound. Carrying r+ from jandem.

Comment 8

[André Bargull [:anba]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=37db99e3318f9f35d0d015dfe7512ab032e8cdc3
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ff3c41880e0d5f992eaff4fcf9362a3bc1b1340

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/22933322f28b
Warn about deprecated Date.prototype.toLocaleFormat method. r=jandem

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/22933322f28b

Comment 11

[Jorg K (CEST = GMT+2)]

André, I was under the impression that a somewhat harmless *warning* message was implemented there.
Sadly this is coming out as JS error and causing us massive test failures: Bug 1345832.
(Unless I'm completely wrong and the test failures come from elsewhere.)

Ah, well, we'll just have to address it quickly as was the intention ;-)

Comment 12

[André Bargull [:anba]]

(In reply to Jorg K (GMT+1) from comment #11)
> André, I was under the impression that a somewhat harmless *warning* message
> was implemented there.
> Sadly this is coming out as JS error and causing us massive test failures:
> Bug 1345832.
> (Unless I'm completely wrong and the test failures come from elsewhere.)

This happens when the "werror" context option is set [1]. Without that option, it should only print a warning message, but continue execution. But I guess the test environment sets "werror" to catch additional errors?

[1] http://searchfox.org/mozilla-central/rev/78ac0ceba97bd2deed847a8d0ae86ccf7a8887bf/js/src/jscntxt.cpp#419-421

Comment 13

[Kohei Yoshino [:kohei]]

I'm postponing writing a site compatibility note on this deprecation warning, because, as I wrote in Bug 818634, there's no simple alternative solution on Firefox for Android now. If the Intl API is enabled on Fennec during the 55 development cycle, I'll post a doc shortly.

Comment 14

[Florian Scholz (Open Web Docs)]

Updated https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleFormat

Added https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Deprecated_toLocaleFormat which I plan to add to https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/errordocs.js (with a few other pages I'm writing), so that we offer a [Learn more] link here. Please add your migration tips to this wiki page, if you have any.

Comment 15

[Kohei Yoshino [:kohei]]

Posted the site compatibility note, finally: https://www.fxsitecompat.com/en-CA/docs/2017/date-prototype-tolocaleformat-has-been-deprecated/