As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
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]
:
: Matthew N. [:MattN] (PM me if requests are blocking you)
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 User image 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 User image 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 User image 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 User image Masatoshi Kimura [:emk] 2013-06-03 21:36:32 PDT
Doesn't Deprecated.jsm support localization?
Comment 4 User image 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 User image :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 User image 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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2013-06-24 06:32:45 PDT
Should be able to land this now.
Comment 8 User image 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 User image :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 User image Phil Ringnalda (:philor) 2013-06-24 22:03:53 PDT
https://hg.mozilla.org/mozilla-central/rev/6ec7d5e0e678
Comment 11 User image 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 User image 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 User image 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 User image Ryan VanderMeulen [:RyanVM] 2013-07-02 05:59:24 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/a0836a9de974
Comment 15 User image 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.