nsIFormHistory2 is deprecated and will be removed.

RESOLVED FIXED in Firefox 24

Status

()

Toolkit
Form Manager
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

({addon-compat})

unspecified
mozilla25
addon-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
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)
Attachment #757772 - Flags: review?(mak77)
(Assignee)

Updated

4 years ago
Whiteboard: [leave-open]
(Assignee)

Comment 2

4 years ago
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!
Keywords: addon-compat
Doesn't Deprecated.jsm support localization?
(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!
(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 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.
Attachment #757772 - Flags: review?(mak77) → review+

Updated

4 years ago
Depends on: 878677
Whiteboard: [leave-open] → [leave-open][don't land until bug 878677 is fixed]
Should be able to land this now.
Whiteboard: [leave-open][don't land until bug 878677 is fixed]
(Assignee)

Comment 8

4 years ago
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.
Status: NEW → ASSIGNED
status-firefox24: --- → affected
tracking-firefox24: --- → ?
No need to track, but I'll approve the patch.
tracking-firefox24: ? → ---
https://hg.mozilla.org/mozilla-central/rev/6ec7d5e0e678
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
(Assignee)

Comment 11

4 years ago
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 :)
(Assignee)

Comment 12

4 years ago
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
Attachment #757772 - Flags: approval-mozilla-aurora?

Comment 13

4 years ago
(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!

Updated

4 years ago
Attachment #757772 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/a0836a9de974
status-firefox24: affected → fixed
status-firefox25: --- → fixed
Assuming no QA needed here. Please use the verifyme keyword to flag for QA verification.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.