879118 - nsIFormHistory2 is deprecated and will be removed.

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

nsIFormHistory2 is deprecated as it performs synchronous IO on the main thread.  Code which uses this interface should be updated to use FormHistory.jsm.

This bug will use the Deprecated.jsm module to warn about the deprecation, with the deprecation message pointing at this bug.  See also bug 876002.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Add deprecation warning

This adds the deprecation warning.  I'll add [leave-open] to the whiteboard and only resolve this bug once bug 876002 is closed (I didn't refer to that bug in the deprecation message as there is noise in that bug that probably isn't of interest to people who care about this message such as addon authors)

Comment 2

[Mark Hammond [:markh] [:mhammond]]

This deprecation warning exists mainly as the interface is used extensively by addons, so keywords=>addon-compat and cc=>jorge - I hope that's appropriate!

Comment 3

[Masatoshi Kimura [:emk]]

Doesn't Deprecated.jsm support localization?

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Mark Hammond (:markh) from comment #2)
> This deprecation warning exists mainly as the interface is used extensively
> by addons, so keywords=>addon-compat and cc=>jorge - I hope that's
> appropriate!

Yes, that's exactly the sort of thing we track. Thanks!

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to Masatoshi Kimura [:emk] from comment #3)
> Doesn't Deprecated.jsm support localization?

It doesn't: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Deprecated.jsm

You could file a bug to fix that, but since the target audience is limited to add-on developers, it may not be worth the effort.

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 757772 [details] [diff] [review]
Add deprecation warning

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

The patch looks ok, though I think we can't take it until we fix at least bug 878677, otherwise is going to fire for any Sync user and we'd lose its benefits.
And that may be tricky by knowing Sync :(

::: toolkit/components/satchel/nsFormHistory.js
@@ +18,5 @@
>  
>  function FormHistory() {
> +    Deprecated.warning(
> +        "nsIFormHistory2 is deprecated and will be removed in a future version",
> +        "https://bugzilla.mozilla.org/show_bug.cgi?id=879118");

please also add a @deprecated note to toolkit/components/satchel/nsIFormHistory.idl, suggesting in the comment to use the new module.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Should be able to land this now.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ec7d5e0e678

We'd also like to uplift this to 24 to help addon authors learn about this deprecation as soon as possible.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

No need to track, but I'll approve the patch.

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6ec7d5e0e678

Comment 11

[Mark Hammond [:markh] [:mhammond]]

I'm about to request approval to land this on Aurora, which means bug 886820 is going to start happening there too.  Jim, if this is a problem for you, speak now or forever hold your peace :)

Comment 12

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 757772 [details] [diff] [review]
Add deprecation warning

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a - we want to give addon developers as much time as possible to adjust to pending removal of the synchronous form history api.
User impact if declined: addons may not adjust to using the new async api in a timely fashion
Testing completed (on m-c, etc.): on central
Risk to taking this patch (and alternatives if risky): very low risk - causes a deprecation warning to appear on the console when this deprecated api is used.
String or IDL/UUID changes made by this patch: None

Comment 13

[Jim Mathies [:jimm]]

(In reply to Mark Hammond (:markh) from comment #11)
> I'm about to request approval to land this on Aurora, which means bug 886820
> is going to start happening there too.  Jim, if this is a problem for you,
> speak now or forever hold your peace :)

We're not on aurora yet, so not an issue. Thanks for checking!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0836a9de974

Comment 15

[u279076]

Assuming no QA needed here. Please use the verifyme keyword to flag for QA verification.