Closed Bug 189229 Opened 22 years ago Closed 17 years ago

xembed for plugins

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: blizzard, Assigned: blizzard)

References

Details

Attachments

(2 files, 4 obsolete files)

It's time to start allowing plugins to use xembed instead of just Xt.  We should
also expose the toolkit being used by the browser so that plugins like flash,
which know about more than one toolkit, can take advantage.

(I'm getting sick and tired of focus bugs.)

I'll start writing this.
I have an idea to rewrite the mozilla part xembed supporting code. To deal with
xembed event in nsWindow instead of give the right to gtksocket. Then we can
integrate plugin focus into current mozilla focus machanism seamlessly. I might
be wrong at this point.

On the other hand, we still can do nothing with those plugins (all of them???)
that does not support xembed. Currently, the focus issue is not because we don't
support xembed, it is because they dont support it. :(
We don't hand plugins an xembed window.  We always hand them an Xt widget
window.  I was planning to pass plugins that advertise that they support xembed
an xembed window (really the window for a gtksocket) and let them work it out.
I'd like to repeat my opnion in
http://bugzilla.mozilla.org/show_bug.cgi?id=187280#c5 here.

XEMBED is only meanful to the toolkit that uses the global focus management in a
logical way, which is described in the xembed protocol spec. It's more than that
embeder and plugger communicate with xembed. It needs the support from the
toolkit level.
The java awt team at sun has a plan to support xembed for java plugin and has
implemented part of it. 
We also have some work to do on browser side as you know java plugin does not
use gtkxtbin. To make mozilla support xembed in a more general way, my plan is
to consider the x window of NS_NATIVE_PLUGIN_PORT as the embedder and the child
of it which is created by plugin or gtkxtbin as the client. We could use
GtkSocket to wrap that embedder window but I think it is more reasonalble to
deal with the protocol ourselves, which makes it much easier to cooperate with
nsWidget to process focus/activate/resize...
Blizzard, what's your opinion?
I need to get the patch off my other machine.  *sigh*

Anyway, I extended the NS plugin API to include two options:

1. Mozilla can query the plugin if it supports XEmbed.  If it does, Mozilla will
create a gtksocket for the plugin to use.  The widget code is not involved with
this at all.

2. The plugin can query Mozilla to see what toolkit is currently in use so it
can integrate directly with the mainloop instead of having to fire up the mainloop.

All of this hacking is down in the plugin code.  I haven't touched widget/ yet
and I had some example plugins working.  The code wasn't done and I've been
working on other things.
Could you let me have a look at your patch? And here's a question, if a plugin
want to support both xembed/non-xembed browser, is your way still applicable?

My way is hard for the plugins which use gtk2xtbin because both the embedder and
client window are created by browser. Xembed in this situation is like a man
talk to himself and has no use to create the communication between browser and
plugin.
My way works exactly the same for old plugins.  There are no changes there.
This is the first pass at a patch that implements this functionality.  It
creates the socket directly from the plugin instance code.  It will also do
things like crash on shutdown since it's not done yet.
A little more on this patch.

It adds a couple of new values to the NP api.  These include:

o A plugin can say if it requires Xt.  (Actually, this is here so if a plugin
doesn't require Xt it can say no.  For the sake of backwards compatibility, it
defaults to "yes" when the plugin doesn't give any kind of response.)

o A plugin can say that it requires XEmbed.  A plugin that doesn't respond to
this will not use XEmbed for the sake of backward compatibility.

o A plugin can determine what toolkit the browser is using so it can figure out
which toolkit to use for the mainloop.  The macromedia flash plugin actually can
load more than one toolkit to fire up the mainloop.  This is here for their sake
and to help other people move to that model, or at least something other than
what we have now which is event starvation in most cases.

If a plugin does support the XEmbed protocol, a gtksocket is created and the X
window of the socket is passed via the window memeber of the nsPluginWindow
structure that is passed down to the plugin.  It's pretty simple and it's
backwards compatible.
Blizzard, 

Your patch is a good approach for the plugin who uses ns4xPluginInstance and
does not override the ns4xPluginInstance::SetWindow. But acctually
ns4xPluginInstance in on the plugin side and there's no garantee for the plugin
instance derived from it and doesn't override the SetWindow. As far as I know,
there are two plugin does not use it: the default plugin and java plugin.
Although the default plugin is trivial but it is an example that plugin may not
use what we have provided.
I think you confuse things. Nobody is supposed to derive from
|ns4xPluginInstance| and this is _not_ on the plugin side. Mozilla code was
designed for XPCOM plugins which were supposed to implement |nsIPluginInstance|
interface, so to keep backward compatibility with 'old style' plugin sort of
shim layer had been introduced -- |ns4xPluginInstance|. Now XPCOM plugin API is
deprecated and 'old style' plugins are no longer 'old', so they expect plain
|NPP_SetWindow| call. The patch does just that, as I can see.
av, thanks for your explaination. 

But the patch might not provide a plain call of NPP_SetWindow. It just moved the
code to another function |ToolkitSetWindow| and add xembed support to it.

My concern is that adding xembed support code in ns4xPluginInstance.cpp can not
cover all the plugins. It only covers the 4x plugin which does not support
nsIPluginInstance. The plugins which have implement nsIPluignInstance and newer
plugin after xpcom plugin deprecated are all excluded. Is my understanding right?
Are there any plugins out there other than the Java plugin that don't use the 4x
plugin interface?
RealPlayer is an XPCOM plugin, but as far as I know they don't have Unix version.

Robin: yes, the patch does not cover XPCOM plugins (and it does not need to, as
the API is deprecated), moreover, it will work to a full advantage only with
newly compiled plugins because it introduces new variables for NPP_GetValue.
We should probably find a way to make it possible for java to use xembed,
though.  Mabye through some extension specific for that plugin since sun is
probably unwilling to change it.
Yeah, we should probably support this through the XPCOM plugin API so OJI will
get it. 

Not too many changes need to happen though. With XPCOM plugins, the browser can
call nsIPlugin::GetValue or nsIPluginInstance::GetValue to find out information
about a plugin like what's its description is and if it's windowless.

I think the best place for this code to avoid all the #ifdefs and get both XPCOM
and NPAPI plugins might be in something like nsPluginNativeWindowGTK.cpp
(nsPluginNativeWindowGTK2.cpp) like was done for other platforms. With the
|CallSetWindow| virtual method you add code to hook before and after calling
nsIPluginInstance::SetWindow.
Attached patch proposal patch (obsolete) — Splinter Review
I followed Peter's suggestion and created a new file
nsPluginNativeWindowGtk2.cpp. In this file, a gtksocket can be created before
the call of SetWindow. In ns4xPluginInstance::SetWindow, if a gtksocket is
found to be of the ported window, no mXtBin will be created. The result is much
like the patch from Blizzard but also covers xpcom plugins.

If you think it is a right way to handle it, I will manage to finish it.
I found the first call of ns4xPluginInstance::SetWindow is actually being called
by nsPluginHostImpl::InstantiateEmbededPlugin when I am viewing a flash. So, it
seems that adding the code in nsPluginNativeWindow does not work. We need to
create the gtksocket in the first time when the SetWindow is called. Any suggestion?
Attached patch modified proposal patch (obsolete) — Splinter Review
I modified my proposal patch a little to make sure the
nsPluginNativeWindow::CallSetWindow being called in the proper time.

Sorry for the confusion and spam.
Attachment #123742 - Attachment is obsolete: true
Your approach seems a lot cleaner than my path where I was hacking the code
directly.  I would say that you should continue.  Can you make sure that you add
the toolkit callback as I had in my patch?  I thought that would be very useful
for plugin writers in our crazy unix world.

Also, I've got comments for the patch.  Do you want me to wait until you are
ready or do you want me to start now?
Comment on attachment 123774 [details] [diff] [review]
modified proposal patch

This looks good. 

There's no need for the  +#ifndef MOZ_WIDGET_GTK2 in nsPluginHostImpl.cpp: all
these calls should go through CallSetWindow on all toolkits. Also, add an entry
in the nsPluginInstanceVariable enum for XPCOM plugins.
Attached patch patch (obsolete) — Splinter Review
In this patch, I added the toolkit query, removed the MOZ_WIDGET_GTK2 in
nsPluginHostImpl and added new value in nsPluginInstanceVariable.

I also added a new value from NPN side to let the plugins can query if the
browser supports xembed.

I am waiting for your comments now.
Attachment #123774 - Attachment is obsolete: true
Comment on attachment 123854 [details] [diff] [review]
patch

>Index: base/public/npapi.h
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/public/npapi.h,v
>retrieving revision 3.31
>diff -u -r3.31 npapi.h
>--- base/public/npapi.h	2 Apr 2003 22:44:47 -0000	3.31
>+++ base/public/npapi.h	21 May 2003 03:52:30 -0000
>@@ -387,7 +387,9 @@
> 
>   /* 12 and over are available on Mozilla builds starting with 0.9.9 */
>   NPPVjavascriptPushCallerBool = 12,
>-  NPPVpluginKeepLibraryInMemory = 13   /* available in Mozilla 1.0 */
>+  NPPVpluginKeepLibraryInMemory = 13,   /* available in Mozilla 1.0 */
>+  NPPVpluginNeedsXEmbed         = 14,
>+  NPPVpluginNeedsXt             = 15

We're not actually using this yet.  The idea here is that we spin up the Xt
mainloop if the plugin uses Xt.  This might require some changes to the xtbin
code.

> } NPPVariable;
> 
> /*
>@@ -404,7 +406,9 @@
>   /* 10 and over are available on Mozilla builds starting with 0.9.4 */
>   NPNVserviceManager = (10 | NP_ABI_MASK),
>   NPNVDOMElement     = (11 | NP_ABI_MASK),   /* available in Mozilla 1.2 */
>-  NPNVDOMWindow      = (12 | NP_ABI_MASK)
>+  NPNVDOMWindow      = (12 | NP_ABI_MASK),
>+  NPNVToolkit        = (13 | NP_ABI_MASK),
>+  NPNVSupportXEmbedBool = 14

"Supports" not "Support"

> } NPNVariable;
> 
> /*
>Index: base/public/nsplugindefs.h
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/public/nsplugindefs.h,v
>retrieving revision 1.27
>diff -u -r1.27 nsplugindefs.h
>--- base/public/nsplugindefs.h	2 Apr 2003 22:44:48 -0000	1.27
>+++ base/public/nsplugindefs.h	21 May 2003 03:52:31 -0000
>@@ -188,7 +188,8 @@
> 
> enum nsPluginManagerVariable {
>     nsPluginManagerVariable_XDisplay                 = 1,
>-    nsPluginManagerVariable_XtAppContext             = 2
>+    nsPluginManagerVariable_XtAppContext             = 2,
>+    nsPluginManagerVariable_SupportXEmbed            = 14

"Supports" not "Support".  Also, is there any reason for the jump from 2 to 14?
 That seems inconsistent.

> };
> 
> enum nsPluginInstancePeerVariable {
>@@ -203,7 +204,8 @@
>     nsPluginInstanceVariable_DoCacheBool             = 5,
>     nsPluginInstanceVariable_CallSetWindowAfterDestroyBool = 6,
>     nsPluginInstanceVariable_ScriptableInstance      = 10,
>-    nsPluginInstanceVariable_ScriptableIID           = 11
>+    nsPluginInstanceVariable_ScriptableIID           = 11,
>+    nsPluginInstanceVariable_NeedsXEmbed             = 14
> };
> 
> ////////////////////////////////////////////////////////////////////////////////
>Index: base/src/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/Makefile.in,v
>retrieving revision 1.90
>diff -u -r1.90 Makefile.in
>--- base/src/Makefile.in	25 Apr 2003 01:47:41 -0000	1.90
>+++ base/src/Makefile.in	21 May 2003 03:52:31 -0000
>@@ -88,8 +88,13 @@
> 	CPPSRCS += nsPluginsDirDarwin.cpp
> 	CPPSRCS += nsPluginNativeWindow.cpp
> else
>+ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>+	CPPSRCS += nsPluginsDirUnix.cpp
>+	CPPSRCS += nsPluginNativeWindowGtk2.cpp
>+else
> 	CPPSRCS += nsPluginsDirUnix.cpp
> 	CPPSRCS += nsPluginNativeWindow.cpp
>+endif
> endif
> endif
> endif
>Index: base/src/ns4xPlugin.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/ns4xPlugin.cpp,v
>retrieving revision 1.99
>diff -u -r1.99 ns4xPlugin.cpp
>--- base/src/ns4xPlugin.cpp	2 Apr 2003 22:44:48 -0000	1.99
>+++ base/src/ns4xPlugin.cpp	21 May 2003 03:52:31 -0000
>@@ -1230,6 +1230,17 @@
> #if defined(XP_UNIX) && !defined(XP_MACOSX)
>   case NPNVxDisplay : {
> #if defined(MOZ_WIDGET_GTK) || defined(MOZ_WIDGET_GTK2)
>+#ifdef MOZ_WIDGET_GTK2

Do you want to move this MOZ_WIDGET_GTK2 code above the #ifdef so that they
aren't nested?

>+    if(npp) {
>+			ns4xPluginInstance *inst = (ns4xPluginInstance *) npp->ndata;
>+			NPBool rtv = PR_FALSE;
>+			inst->GetValue((nsPluginInstanceVariable)NPPVpluginNeedsXEmbed, &rtv);
>+			if(rtv) {
>+				(*(Display **)result) = GDK_DISPLAY();
>+				return NPERR_NO_ERROR;
>+			}
>+    }
>+#endif

There seem to be some tabs here.

>     // adobe nppdf calls XtGetApplicationNameAndClass(display, &instance, &class)
>     // we have to init Xt toolkit before get XtDisplay
>     // just call gtk_xtbin_new(w,0) once
>@@ -1340,6 +1351,31 @@
>       }
>     }
>     return NPERR_GENERIC_ERROR;
>+  }
>+
>+  case NPNVToolkit: {
>+    result = NULL;
>+
>+#ifdef MOZ_WIDGET_GTK
>+    (const char *)result = "gtk12";
>+#endif
>+
>+#ifdef MOZ_WIDGET_GTK2
>+    (const char *)result = "gtk2";
>+#endif
>+
>+    if (result)
>+        return NPERR_NO_ERROR;
>+
>+    return NPERR_GENERIC_ERROR;
>+  }
>+
>+  case NPNVSupportXEmbedBool: {
>+    *(NPBool*)result = PR_FALSE; 
>+#ifdef MOZ_WIDGET_GTK2
>+    *(NPBool*)result = PR_TRUE; 
>+#endif
>+    return NPERR_NO_ERROR;

How about:

#ifdef MOZ_WIDGET_GTK2
PR_TRUE
#else
PR_FALSE

>   }
>   default : return NPERR_GENERIC_ERROR;
>   }
>Index: base/src/ns4xPluginInstance.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/ns4xPluginInstance.cpp,v
>retrieving revision 1.107
>diff -u -r1.107 ns4xPluginInstance.cpp
>--- base/src/ns4xPluginInstance.cpp	24 Mar 2003 04:15:12 -0000	1.107
>+++ base/src/ns4xPluginInstance.cpp	21 May 2003 03:52:31 -0000
>@@ -869,6 +869,7 @@
>   NPError error;
>   
> #if defined (MOZ_WIDGET_GTK) || defined (MOZ_WIDGET_GTK2)
>+  PRBool isXembed = PR_FALSE;
>   // bug 108337, flash plugin on linux doesn't like window->width <= 0
>   if ((PRInt32) window->width <= 0 || (PRInt32) window->height <= 0)
>     return NS_OK;
>@@ -890,6 +891,15 @@
>     GdkWindow *win = gdk_window_lookup((XID)window->window);
>     if (!win)
>       return NS_ERROR_FAILURE;
>+    gpointer user_data = nsnull;
>+    gdk_window_get_user_data(win, &user_data);
>+    if(user_data) {
>+      GtkWidget* widget = GTK_WIDGET(user_data);
>+      if(GTK_IS_SOCKET(widget)) {
>+	isXembed = PR_TRUE;
>+      }
>+    }
>+    if(!isXembed)
>     {  
> #ifdef NS_DEBUG      
>       printf("About to create new xtbin of %i X %i from %p...\n",
>@@ -938,7 +948,12 @@
>     ws->type = 0; // OK, that was a guess!!
> #ifdef MOZ_X11
>     ws->depth = gdk_window_get_visual(win)->depth;
>-    ws->display = GTK_XTBIN(mXtBin)->xtdisplay;
>+    if(!isXembed) {
>+      ws->display = GTK_XTBIN(mXtBin)->xtdisplay;
>+    }
>+    else {
>+      ws->display = GDK_WINDOW_XDISPLAY(win);
>+    }

Single line if/else shouldn't use braces:

if (!isXembed)
    ws->display = GTK_XTBIN(mXtBin)->xtdisplay;

etc

>     ws->visual = GDK_VISUAL_XVISUAL(gdk_window_get_visual(win));
>     ws->colormap = GDK_COLORMAP_XCOLORMAP(gdk_window_get_colormap(win));
> 
>@@ -946,14 +961,16 @@
> #endif
>   } // !window->ws_info
> 
>-  if (!mXtBin)
>+  if (!mXtBin&&!isXembed)

Please add some whitespace here.

if (!mXtBin && !isXembed)

>     return NS_ERROR_FAILURE;
> 
>-  // And now point the NPWindow structures window 
>-  // to the actual X window
>-  window->window = (nsPluginPort *)GTK_XTBIN(mXtBin)->xtwindow;
>-  
>-  gtk_xtbin_resize(mXtBin, window->width, window->height);
>+  if(!isXembed) {

Bad whitespace here, too:

if (!isXEmbed) {

>+    // And now point the NPWindow structures window 
>+    // to the actual X window
>+    window->window = (nsPluginPort *)GTK_XTBIN(mXtBin)->xtwindow;
>+    
>+    gtk_xtbin_resize(mXtBin, window->width, window->height);
>+  }
>   
> #elif defined(MOZ_WIDGET_XLIB)
> 
>Index: base/src/nsPluginHostImpl.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp,v
>retrieving revision 1.475
>diff -u -r1.475 nsPluginHostImpl.cpp
>--- base/src/nsPluginHostImpl.cpp	5 May 2003 21:27:02 -0000	1.475
>+++ base/src/nsPluginHostImpl.cpp	21 May 2003 03:52:32 -0000
>@@ -2101,7 +2101,10 @@
>           // If we've got a native window, the let the plugin know
>           // about it.
>           if (window->window)
>-            mInstance->SetWindow(window);
>+	  {
>+	    nsCOMPtr<nsIPluginInstance> inst = mInstance;
>+            ((nsPluginNativeWindow*)window)->CallSetWindow(inst);
>+	  }

Are there some tabs here, too?	Looks like the bracing is actually left of the
if() that it's attached to.  Lots of this below, too.


> #endif
>+  if (nsPluginManagerVariable_SupportXEmbed == aVariable) {
>+    *(NPBool*)aValue = PR_FALSE;
>+#ifdef MOZ_WIDGET_GTK2
>+    *(NPBool*)aValue = PR_TRUE;
>+#endif
>+  }

Same PR_TRUE/PR_FALSE complaint as above.

>Index: base/src/nsPluginNativeWindowGtk2.cpp
>===================================================================
>--- /dev/null	2002-08-31 07:31:37.000000000 +0800
>+++ ./base/src/nsPluginNativeWindowGtk2.cpp	2003-05-20 09:52:49.000000000 +0800
>@@ -0,0 +1,151 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* vim:expandtab:shiftwidth=2:tabstop=2:
>+*/
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Netscape Public License
>+ * Version 1.1 (the "License"); you may not use this file except in
>+ * compliance with the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/NPL/

We shouldn't be using the NPL anymore.	Where did you get this header?

>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is mozilla.org code.
>+ *
>+ * The Initial Developer of the Original Code is 
>+ * Netscape Communications Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 1998
>+ * the Initial Developer. All Rights Reserved.

Isn't this Sun code?

>+ *
>+ * Contributor(s):
>+ *   Andrei Volkov <av@netscape.com>
>+ *   Brian Stell <bstell@netscape.com>
>+ *   Peter Lubczynski <peterl@netscape.com>

And did these people really write this code?

>+nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsCOMPtr<nsIPluginInstance> &aPluginInstance)

Wow, shouldn't this be:

nsIPluginInstance *aPluginInstance

I don't think I've ever seen anyone use an nsCOMPtr<> in a function signature.
>>+ * Contributor(s):
>>+ *   Andrei Volkov <av@netscape.com>
>>+ *   Brian Stell <bstell@netscape.com>
>>+ *   Peter Lubczynski <peterl@netscape.com>

This is probably cut-n-paste thing from Win version of the file.

>And did these people really write this code?

>>+nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsCOMPtr<nsIPluginInstance>
>>&aPluginInstance)

>Wow, shouldn't this be:

>nsIPluginInstance *aPluginInstance

>I don't think I've ever seen anyone use an nsCOMPtr<> in a function signature.

It is not recommended in interface method signatures but other than that passing
nsCOMPtr by reference is described as a preferred way in
http://mozilla.org/projects/xpcom/nsCOMPtr.html (just before Summary).
>>+  NPPVpluginNeedsXt             = 15
> 
> 
> We're not actually using this yet.  The idea here is that we spin up the Xt
> mainloop if the plugin uses Xt.  This might require some changes to the xtbin
> code.
I removed it currently. We could implement in another bug.
>>+    nsPluginManagerVariable_XtAppContext             = 2,
>>+    nsPluginManagerVariable_SupportXEmbed            = 14
> 
> 
> "Supports" not "Support".  Also, is there any reason for the jump from 2 to 14?
>  That seems inconsistent.
The "NPNVSupportXEmbedBool" in npapi.h is 14 and I think they should be same.
>>+nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsCOMPtr<nsIPluginInstance>
&aPluginInstance)
> 
> 
> Wow, shouldn't this be:
> 
> nsIPluginInstance *aPluginInstance
> 
> I don't think I've ever seen anyone use an nsCOMPtr<> in a function signature.
> 
The signature is defined in nsPluginNativeWindow.h.
Attached patch patch (obsolete) — Splinter Review
I modified the patch according to Blizzard's comments. I also added the code in
nsWindow.cpp to avoid the non-xembed plugin focus treatment interfering xembed
one.
Attachment #123854 - Attachment is obsolete: true
Attached patch patchSplinter Review
This patch includes some bug fixes from last patch
Attachment #123970 - Attachment is obsolete: true
Blocks: 120921
Comment on attachment 126518 [details] [diff] [review]
patch

This patch has been tested by the our experimental xembed java plugin and works
well. Blizzard, could you take time to review the patch? Thanks!
Attachment #126518 - Flags: review?(blizzard)
Comment on attachment 126518 [details] [diff] [review]
patch

r=blizzard
Attachment #126518 - Flags: review?(blizzard) → review+
Attachment #126518 - Flags: superreview?(beard)
Comment on attachment 126518 [details] [diff] [review]
patch

Patrick Beard no longer works for mozilla. Who can I seek the sr from?
Attachment #126518 - Flags: superreview?(beard)
Attachment #126518 - Flags: superreview?(brendan)
Attachment #126518 - Flags: superreview?(brendan)
Attachment #126518 - Flags: superreview+
Attachment #126518 - Flags: review+
Attachment #126518 - Flags: review?(joshua.xia)
Comment on attachment 126518 [details] [diff] [review]
patch

r=joshua
Attachment #126518 - Flags: review?(joshua.xia) → review+
Attachment #126518 - Flags: approval1.5?
Comment on attachment 126518 [details] [diff] [review]
patch

Too late for new features in 1.5. We should get this in to the trunk as soon as
we open for 1.6a.
Attachment #126518 - Flags: approval1.5? → approval1.5-
Checked in trunk.
Thank you all!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
This checkin caused the regression in bug 219705 - crash when encountering a
page with a Java plugin.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
see my comments in bug 219705.
It is not a regression of this patch. The patch just exposed a bug in current
version of java plugin. Do you agree to leave this bug closed? Thanks!
Is this a common bug in all platforms' java plugins?

Are there any publically available Java plugins without the problem (in
both gcc 2.95 and 3.2 forms) that we can point users to?  If there isn't,
this patch needs some work quickly or a backout.
We are working on it now. We will provide a patch to work it around on browser
side as soon as possible.
Blocks: 101082
Adobe is releasing _open source_ code to help web browser developers prepare web browsers to use XEmbed hooks implemented in future flash plugins.

http://blogs.adobe.com/penguin.swf/2007/04/bling.html
Zbigniew: Please stop spamming bugs with that.

I'm not sure why this is still open, so I'm changing that. XEmbed support has been in place for a few years now; and looking at the discussion in this bug, it's not clear that there's anything more to be done on the Mozilla side.
Status: REOPENED → RESOLVED
Closed: 21 years ago17 years ago
Resolution: --- → FIXED
Depends on: 731917
Depends on: 348247
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: