Last Comment Bug 615213 - Remove nsIGlobalHistory (Components.classes["@mozilla.org/browser/global-history;1"]) code
: Remove nsIGlobalHistory (Components.classes["@mozilla.org/browser/global-hist...
Status: RESOLVED FIXED
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
-- major (vote)
: mozilla15
Assigned To: Marco Castelluccio [:marco]
:
: Andrew Overholt [:overholt]
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on: data-driven-compreg
Blocks: asyncHistory 615219
  Show dependency treegraph
 
Reported: 2010-11-29 07:37 PST by Serge Gautherie (:sgautherie)
Modified: 2012-05-15 07:29 PDT (History)
8 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Patch (5.57 KB, patch)
2012-05-03 11:07 PDT, Marco Castelluccio [:marco]
benjamin: review+
Details | Diff | Splinter Review

Description User image Serge Gautherie (:sgautherie) 2010-11-29 07:37:48 PST
http://mxr.mozilla.org/mozilla-central/source/docshell/base/Makefile.in
still has
{
55 SDK_XPIDLSRCS = \
56                 nsIGlobalHistory.idl \
57                 $(NULL)
}

But nsGlobalHistoryAdapter.cpp was removed by bug 568691.
http://hg.mozilla.org/mozilla-central/rev/08410c390aa9

Then it seems this interface is not registered (anywhere) anymore.

1) Was it intended or just missed?
2) Are there other interfaces in the same case?
Comment 1 User image Serge Gautherie (:sgautherie) 2010-11-29 07:43:10 PST
Fwiw,
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsIGlobalHistory.idl
42  * @status DEPRECATED. This interface is still accepted, but new
43  *         implementations should use nsIGlobalHistory2.

Then you might want to take Gecko 2.0 as an opportunity to remove this interface completely.
But that's just a reminder, not a request.
Comment 2 User image Benjamin Smedberg [:bsmedberg] 2010-11-29 07:45:58 PST
This was an intentional change. We should remove the interface.
Comment 3 User image Serge Gautherie (:sgautherie) 2010-11-29 08:19:50 PST
"blocking2.0=?":
Stop exposing/building an interface that is actually unavailable now.
Comment 4 User image Benjamin Smedberg [:bsmedberg] 2010-11-29 08:20:53 PST
No, I don't want to remove it now because extension code which doesn't actually use it may still be trying to use Components.classes.nsIGlobalHistory.
Comment 5 User image Serge Gautherie (:sgautherie) 2010-11-29 08:39:12 PST
(In reply to comment #2)
> We should remove the interface.

(In reply to comment #4)
> I don't want to remove [the code]

Wow, I'm puzzled!
So you say you want to "keep (building) the code in the tree, yet not register the interface anymore"?

(I wonder what good that does.
Maybe take bug 603031 + bug 615219 as an example.)
Comment 6 User image Benjamin Smedberg [:bsmedberg] 2010-11-29 08:49:07 PST
No, I'm saying we should remove the interface *after* we branch for Firefox 4. The *interface* is still present in Firefox 4, but there are no implementations of it, so it's vestigial.
Comment 7 User image Serge Gautherie (:sgautherie) 2010-11-29 09:12:24 PST
(In reply to comment #6)

Hum (I'm not sure I fully understand, but), in other words, am I right to say that:
*this bug is Future for now (until FFv4 branches)?
*ChatZilla is now required to use nsIGlobalHistory2 (bug 615219) to keep working with Gecko 2.0b2+?
Comment 8 User image Marco Bonardo [::mak] 2012-05-03 09:56:22 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> No, I'm saying we should remove the interface *after* we branch for Firefox
> 4.

So, we are quite far from Firefox 4, we still don't have an implementation of this interface, so whoever tries to use it is likely just not doing what he thinks.

Benjamin, would you be fine to actually do the removal now?  We can reach tracked add-ons authors through the AMO team, though at this point I expect those being just onowned and unmaintained add-ons.
Comment 9 User image Benjamin Smedberg [:bsmedberg] 2012-05-03 10:32:03 PDT
Yes, please remove.
Comment 10 User image Marco Castelluccio [:marco] 2012-05-03 11:07:38 PDT
Created attachment 620783 [details] [diff] [review]
Patch

Remove nsIGlobalHistory (if we want to)
Comment 11 User image Marco Bonardo [::mak] 2012-05-07 13:30:37 PDT
I assume benjamin's review here is also valid as a SR for the interface change, and after all this was already in the plan from a long time.  So this can land.
Comment 12 User image Marco Castelluccio [:marco] 2012-05-09 01:08:41 PDT
I've sent the patch to try: https://tbpl.mozilla.org/?tree=Try&rev=fce464dcb502
Comment 14 User image Ed Morley [:emorley] 2012-05-10 07:40:52 PDT
https://hg.mozilla.org/mozilla-central/rev/c0110c112580
Comment 15 User image Eric Shepherd [:sheppy] 2012-05-15 07:29:38 PDT
Doc updated:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIGlobalHistory

Mentioned on Firefox 15 for developers.

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