Closed Bug 1313625 Opened 3 years ago Closed 2 years ago

Remove nsIScriptableDateFormat [was: Deprecate nsIScriptableDateFormat]

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jfkthame, Assigned: emk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

From bug 1301655 comment 0:
> We shouldn't need to have an XPCOM API like nsIScriptableDateFormat for browser
> UI JS to format dates and times with. Now that the ECMAScript Intl API is
> available, UI JS code should use the ECMAScript Intl API instead of
> nsIScriptableDateFormat so that the nsIScriptableDateFormat legacy API could
> be removed.

That bug is eliminating the existing users of the API in mozilla-central. As there will doubtless be other users as well (comm-central, add-ons, ...), we should explicitly deprecate the interface and point people (through documentation, maybe a console warning?) towards the modern alternative.
:jorgev, can we check for code that uses nsIScriptableDateFormat in add-ons, and prompt the authors to migrate to Intl.DateTimeFormat instead?
Flags: needinfo?(jorge)
I see a number of add-ons using it. The ideal process in this case is to first deprecate the API and show a console warning when it's used. A release or two later, it can be dropped entirely.

We can message the developers for both events.
Flags: needinfo?(jorge)
Keywords: addon-compat
This gives us a build-time warning when nsIScriptableDateFormat is compiled (at least on OS X; I'm not sure if all our compilers do that), and a simple runtime message in the browser console when it's used. I didn't make the message localizable, as that seemed like overkill here; I don't think we need to burden localizers with this.
Attachment #8805914 - Flags: review?(gandalf)
Attachment #8805914 - Flags: review?(gandalf) → review+
Depends on: 1346549
Bug 1354442 is the last in Firefox that uses nsIScriptableDateFormat.
Depends on: 1354442
Depends on: 1376167
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Hmm, looks like we never landed the patch (attachment 8805914 [details] [diff] [review]) that was going to add a deprecation warning. :-( I guess it's a bit late for that now.

Still, maybe we should land it here, and then have a followup in a few weeks to actually remove the code. WDYT?
Flags: needinfo?(VYV03354)
I think we should not give an illusion that we still keep stability of the internal API even after 57.
Flags: needinfo?(VYV03354)
We removed nsIScriptableDateFormat from C-C, so you can remove it any time ;-)
Comment on attachment 8893675 [details]
Bug 1313625 - Remove nsIScriptableDateFormat.

https://reviewboard.mozilla.org/r/164776/#review170254

Fine, let's do it.
Attachment #8893675 - Flags: review?(jfkthame) → review+
Attachment #8805914 - Attachment is obsolete: true
Summary: Deprecate nsIScriptableDateFormat → Remove nsIScriptableDateFormat [was: Deprecate nsIScriptableDateFormat]
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 2b990ca29826 -d 76dcd1e6797b: rebasing 411597:2b990ca29826 "Bug 1313625 - Remove nsIScriptableDateFormat. r=jfkthame" (tip)
local [dest] changed intl/locale/nsIScriptableDateFormat.idl which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed intl/locale/nsScriptableDateFormat.cpp which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
merging intl/build/nsI18nModule.cpp
merging intl/locale/moz.build
merging intl/locale/nsLocaleConstructors.h
warning: conflicts while merging intl/locale/moz.build! (edit, then use 'hg resolve --mark')
warning: conflicts while merging intl/locale/nsLocaleConstructors.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/afc4e2d4eb96
Remove nsIScriptableDateFormat. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/afc4e2d4eb96
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
We don't care about (legacy) addon-compat anymore.
Depends on: 1389449
You need to log in before you can comment on or make changes to this bug.