Last Comment Bug 879118 - nsIFormHistory2 is deprecated and will be removed.
: nsIFormHistory2 is deprecated and will be removed.
Status: RESOLVED FIXED
[qa-]
: addon-compat
Product: Toolkit
Classification: Components
Component: Form Manager (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla25
Assigned To: Mark Hammond [:markh]
:
Mentors:
Depends on: 878677
Blocks: killSyncFormHistory
  Show dependency treegraph
 
Reported: 2013-06-03 19:40 PDT by Mark Hammond [:markh]
Modified: 2013-09-13 15:01 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Add deprecation warning (1.36 KB, patch)
2013-06-03 20:00 PDT, Mark Hammond [:markh]
mak77: review+
bajaj.bhavana: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Mark Hammond [:markh] 2013-06-03 19:40:59 PDT
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] 2013-06-03 20:00:11 PDT
Created attachment 757772 [details] [diff] [review]
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] 2013-06-03 20:07:50 PDT
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] 2013-06-03 21:36:32 PDT
Doesn't Deprecated.jsm support localization?
Comment 4 Jorge Villalobos [:jorgev] 2013-06-04 09:50:01 PDT
(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: gavin@gavinsharp.com] 2013-06-04 09:55:46 PDT
(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] 2013-06-05 13:49:53 PDT
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: gavin@gavinsharp.com] 2013-06-24 06:32:45 PDT
Should be able to land this now.
Comment 8 Mark Hammond [:markh] 2013-06-24 10:23:31 PDT
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: gavin@gavinsharp.com] 2013-06-24 10:36:44 PDT
No need to track, but I'll approve the patch.
Comment 10 Phil Ringnalda (:philor) 2013-06-24 22:03:53 PDT
https://hg.mozilla.org/mozilla-central/rev/6ec7d5e0e678
Comment 11 Mark Hammond [:markh] 2013-06-26 12:40:00 PDT
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] 2013-06-26 12:42:24 PDT
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] 2013-06-26 12:54:23 PDT
(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] 2013-07-02 05:59:24 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/a0836a9de974
Comment 15 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-09-13 15:01:46 PDT
Assuming no QA needed here. Please use the verifyme keyword to flag for QA verification.

Note You need to log in before you can comment on or make changes to this bug.