Closed Bug 1315812 Opened 3 years ago Closed 3 years ago

Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

Details

(Keywords: addon-compat)

Attachments

(1 file, 1 obsolete file)

This marks the idl classes as deprecated, removes an unnecessary include that
was triggering deprecation warnings and wraps a necessary include in
XPCOMInit.cpp that is used for registering the component in deprecation
disabling pragmas.

MozReview-Commit-ID: BbNU5q8O4Q4
Comment on attachment 8808395 [details] [diff] [review]
Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated

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

r=me, I guess, though I'd like to understand why we only need the single deprecation location.

::: xpcom/build/XPCOMInit.cpp
@@ +44,5 @@
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +#elif defined(_MSC_VER)
> +#pragma warning (push)
> +#pragma warning (disiable : 4996)

"disiable"? :)

@@ +51,1 @@
>  #include "nsSupportsArray.h"

How is this the only place that requires the deprecation suppression?  Surely the implementation code somewhere includes the deprecated .h files...
Attachment #8808395 - Flags: review?(nfroyd) → review+
Now with more pragmas
Attachment #8809233 - Flags: review?(nfroyd)
Attachment #8808395 - Attachment is obsolete: true
Attachment #8809233 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f424e29dc77ed2790f3ab6e05940a31c4a85316a
Bug 1315812 - Mark nsISupportsArray, nsICollection, nsIEnumerator as deprecated. r=froydnj
Keywords: addon-compat
https://hg.mozilla.org/mozilla-central/rev/f424e29dc77e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1317040
It could be better for all external users if this went into 53. Forcing changing of interfaces (due to compiler warnings-as-errors) so close before branching of 52 ESR is not nice.
(In reply to :aceman from comment #6)
> It could be better for all external users if this went into 53. Forcing
> changing of interfaces (due to compiler warnings-as-errors) so close before
> branching of 52 ESR is not nice.

You can apply the workarounds for C++ from this very bug; you don't have to change any interfaces.
We did ;-)

Nathan, when will M-C remove nsISupportsArray completely? We still use it extensively in Thunderbird, despite our best ongoing efforts to remove it. It's mainly used in the search component.
Flags: needinfo?(nfroyd)
(In reply to Jorg K (GMT+1) from comment #8)
> We did ;-)
> 
> Nathan, when will M-C remove nsISupportsArray completely? We still use it
> extensively in Thunderbird, despite our best ongoing efforts to remove it.
> It's mainly used in the search component.

I believe Eric was planning on removing it after a single release advertising its deprecation, so in 53.  Is that correct, Eric?  I suppose we could consider other options, especially since it just got more inconvenient to use from C++...
Flags: needinfo?(nfroyd) → needinfo?(erahm)
(In reply to Nathan Froyd [:froydnj] from comment #9)
> (In reply to Jorg K (GMT+1) from comment #8)
> > We did ;-)
> > 
> > Nathan, when will M-C remove nsISupportsArray completely? We still use it
> > extensively in Thunderbird, despite our best ongoing efforts to remove it.
> > It's mainly used in the search component.
> 
> I believe Eric was planning on removing it after a single release
> advertising its deprecation, so in 53.  Is that correct, Eric?  I suppose we
> could consider other options, especially since it just got more inconvenient
> to use from C++...

I think kmag and I settled on a full release cycle so that addon devs can put out one release that works with all of ours, so remove when 52 (or is it 53?) hits release. I'd be happy to remove earlier of course!

If we can't remove the dependencies in TB by that time then another option is to move the interface into c-c at that point.
Flags: needinfo?(erahm)
You need to log in before you can comment on or make changes to this bug.