Closed Bug 569948 Opened 10 years ago Closed 10 years ago

Remove NS_InitEmbedding

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(1 file)

NS_InitEmbedding has been obsoleted by XRE_InitEmbedding, but we'll occasionally see people in the newsgroups still trying to use it. It happens to be in the way of my XPCOM data registration rewrite, so I'm going to remove the footgun.
Attachment #449048 - Flags: superreview?(dougt)
Attachment #449048 - Flags: review?(mark.finkle)
NS_InitEmbedding has been frozen for almost ever.  The plan was to break it for "2.0".  Two questions:

What is the motivation for doing it now?  Does having it exist somehow block you?

What embeddings will we break?  A quick search on http://www.google.com/codesearch does show a few projects using this API.
It makes refactoring component loading in bug 568691 much harder (which is the bug for making content process startup fast in Fennec). And because we're breaking all existing binary components anyway, I don't see the harm. Finally, NS_InitEmbedding ends up in a static library which is linked to the embedder, which is a terribly confusing design.

Besides which it is untested and unmaintained. I at least have people who test XRE_InitEmbedding.
I notice that the Mozilla ActiveX control still uses NS_InitEmbedding, NS_TermEmbedding and NS_HandleEmbeddingEvent. Any plans to patch that too?
Good question, do we only build that in XULRunner builds?

Anyway, no, I would rather stop building and remove the ActiveX control/plugin (move it into a separate project repo if somebody volunteers to maintain it, as we did with pyxpcom).
(In reply to comment #5)
> Good question, do we only build that in XULRunner builds?

Yes, and only Windows (obviously)

> Anyway, no, I would rather stop building and remove the ActiveX control/plugin
> (move it into a separate project repo if somebody volunteers to maintain it, as
> we did with pyxpcom).

Hmm, I remember that Vlad was building the activex plugin wrapper for Firefox and Fennec running on the nVidia Tegra devices as a way to run Flash. However, that project is not active anymore.
So where does that leave us with reviews?
Comment on attachment 449048 [details] [diff] [review]
Just remove it, rev. 1

Looks OK to me, but when this lands the Windows XULRunner builds will bust (and Fennec Win32 builds IIRC).

Can we stop building ActiveX by default on Windows XULRunner?
Attachment #449048 - Flags: review?(mark.finkle) → review+
Comment on attachment 449048 [details] [diff] [review]
Just remove it, rev. 1

be sure to notify the .platform.  Also, please update: 

https://wiki.mozilla.org/XPCOM/Bootstrap
Attachment #449048 - Flags: superreview?(dougt) → superreview+
http://hg.mozilla.org/mozilla-central/rev/8b3af57c81e8
http://hg.mozilla.org/mozilla-central/rev/92527a44edfb
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Why were the users of this frozen API not notified before its removal? I don't recall "it's in my way" being among the reasons to remove APIs promised to embedding clients with hundreds of thousands of users.
I have informed Camino devs and others I knew about for *years* that this API was poor and was likely to be removed. This is merely the culmination of several years of activity. You can copy that code into Camino if you still want it; it was statically linked anyway.
Is there a document describing how migrating to XRE_InitEmbedding would work? Because while we may been told for years that embedding was going to change, and every time I asked for details about what the future would be the answer was essentially "well, probably XULRunner, but it's still changing, so you should wait for that to stabilize and not worry about it right now, it won't matter until Gecko 2 anyway".
What kind of details, other than the comments at http://mxr.mozilla.org/mozilla-central/source/xpcom/build/nsXULAppAPI.h#352 are you looking for?

FWIW, we're making breaking changes now (that's the bug this one blocks), and are treating this release as mozilla 2, though we haven't decided whether to bump the version number.
Depends on: 571768
Timeless brought this bug up on IRC. Apparently there was Nokia management unhappiness about this or another change (XPCOM component registration? I can't tell, but that was a good change -- whether it was advertised sufficiently in advance, I don't know).

Re: comment 15, we did bump the Gecko rv and Mozilla milestone version number, and that was always the plan -- I never supported breaking frozen interfaces without doing that. But that's not enough.

Posting to the relevant newsgroups (yeah, the Google groups archives are spammed now; we're working on a better solution) about a breaking change is only good form and rational self-interest, to avoid blowback like in this bug, and worse. It looks like bsmedberg did that:

http://groups.google.com/group/mozilla.dev.embedding/browse_thread/thread/8c6cdef283a39ba4/f145ecceef8398ae?q=569948+group:mozilla.*

although a bit late.

This bug hasn't had a comment in almost 8 weeks, so I'm going to assume it was a case of failure to notify immediately (even with long-term warnings about the change coming some day), and we will do better next time.

To reiterate something I believe bsmedberg agrees with, making the Mozilla ilestone or Gecko version number change to 2 doesn't mean we can break anything at any time. This is not the Joe-Bob Briggs definition of a horror movie where "anyone can DIE at ANY TIME".

The idea is that frozen interfaces will break where the trade-offs are worth it, and there definitely are cases where it is worth it from everyone's side, among core code developers, embedders, and other devs and users.

Accidents like what happened to nsISelection (bug 581051) will get fixed. "Anything goes" is no good for anyone, developers or embedders. But frozen interfaces are thawing and we will not re-freeze. We won't thrash, either.

/be
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.