121615 - Need netscape 4.x plugin compatibility for gtk2

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Christopher Blizzard (:blizzard)]

Need to implement Netscape 4.x compatibility for gtk2.  The best thing to do is
to probably implement an XEmbed aware Xt widget.

Comment 1

[Christopher Blizzard (:blizzard)]

*** Bug 141081 has been marked as a duplicate of this bug. ***

Comment 2

[Christopher Blizzard (:blizzard)]

*** Bug 141092 has been marked as a duplicate of this bug. ***

Comment 3

[Mindy Liu]

This bug also happens on Solaris 8 with the build: 
Mozilla/5.0(X11;U;SunOS sun4u;en_US;rv:1.0.0+)Gecko/20020508

Comment 4

[Joshua Xia]

Attachment: This is my first patch for porting plugin to GTK2

This patch is not final version
I will accomplish the whole patch later

Comment 5

[Christopher Blizzard (:blizzard)]

It's a good start.  However, as I said in this bug we should really be using an
XEmbed aware Xt widget.  This way we can get focus, key events and out of
process plugins working properly.

Comment 6

[Joshua Xia]

Now I am porting mozilla/widget/src/gtkxtbin to GTK2.
It relate with Xt widget.

I think the main task is to implement Plugin 's "SetWindow" function in GTK2.

But I have no idea about "XEmbed aware Xt widget", Blizzard, Would you like to 
explain it for me or advise me any other module need to be modified?

Thanks!

Comment 7

[Christopher Blizzard (:blizzard)]

We need to move to use a gtksocket for the plugin window so we can use any kind
of window as the child, as long as that child supports XEmbed.  This means that
we can properly support focus and key event passing to and from the browser and
the plugin.

Once that is done, we need to change the Xt shim that's used to support old 4.x
plugins to use XEmbed so that it will properly pass up focus information and key
events.  Here's an old copy of the XEmbed standard, for your information:

http://www.freedesktop.org/standards/xembed.html

I think this the most recent copy.  I'm trying to make sure of that.  There was
some recent changes that fixed a lot of problems and I want to make sure that we
are supporting the correct version.

Comment 8

[Joshua Xia]

Thanks Blizzard!

So I need to port mozilla/embedding/browser/gtk/ to GTK2?
at same time I need to port mozilla/widget/src/gtksuperwin to GTK2?

After I port plugin and gtkxtbin, The following error happen, Can you check it
for me?

(<unknown>:11023): Gtk-WARNING **: Calling gtk_widget_realize() on a widget that
 isn't inside a toplevel window is not going to work very well. Widgets must be 
inside a toplevel container before realizing them.
......
......
......
The program '<unknown>' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadWindow (invalid Window parameter)'.
  (Details: serial 53648 error_code 3 request_code 10 minor_code 0)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the --sync command line
   option to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)

Comment 9

[Christopher Blizzard (:blizzard)]

embedding/browser/gtk/ doesn't have anything to do with this bug.  Plus, I've
already ported most of it and it's attached to a bug.

gtksuperwin definitly doesn't need to be ported to gtk2.  It's been totally
removed.  You should be able to get the XID passed through the
GetNativeData(NS_NATIVE_PLUGIN_PORT) and use that to create a plug to the
plugin's socket.  Of course, we might need to make some changes to the layout
code and add a call that marks a specific window as a plugin window.  Hmm.. I
need to think about that.

Anyway, this should be possible.  With that in place, we just need an Xt shim
for 4.x plugins that knows how to act in an XEmbed environment.

Comment 10

[Joshua Xia]

Yeah, Thanks!

I have implemented GetNativeData(NS_NATIVE_PLUGIN_PORT), and ported gtkxtbin
and plugin, please review my patch if you have time.

I will check if it is necessary to port layout or not.

Comment 11

[Joshua Xia]

Now by using my patch, the flash and java applet run well.
But the following warning display:

(<unknown>:11023): Gtk-WARNING **: Calling gtk_widget_realize() on a widget that
 isn't inside a toplevel window is not going to work very well. Widgets must be 
inside a toplevel container before realizing them.

and if I switch a html page with Flash to an Applet html page, the browser exit
and display the following error:

The program '<unknown>' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadWindow (invalid Window parameter)'.
  (Details: serial 53648 error_code 3 request_code 10 minor_code 0)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the --sync command line
   option to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)


Blizzard: Can you give me some suggestion to fix this bug?
Thanks!

Comment 12

[Joshua Xia]

Attachment: patch for gtkxtbin

I modified my patch for gtkxtbin.c
Now the plugin(java and flash) work well!

Review it please!
Thanks!

Comment 13

[Joshua Xia]

Attachment: new patch for gtkxtbin.c

modify for second patch

Comment 14

[Joshua Xia]

Attachment: patch for root Makefile.in

This patch make gtkxtbin built before plugin module.

Comment 15

[Christopher Blizzard (:blizzard)]

None of these patches include support for XEmbed.  What's going on there?

Comment 16

[Joshua Xia]

Sorry, patch for root Makefile.in should be:

+ifdef MOZ_ENABLE_GTK2
+tier_9_dirs	+= widget/src/gtkxtbin
+endif
+

needn't include widget/src/gtksuperwin

Comment 17

[Robin Lu]

Attachment: new patch based on Joshua's Patch

It's a patch mostly similar with Joshua's patch. Only arranged and fix a little
bug that makes flash plugin on Linux can work. Flash still crashes due to
BadWindow ( a window destroyed by two times). Still not an XEmbeded patch.
Sorry for that!

Comment 18

[Robin Lu]

Attachment: Plan for XEmbed Plugin

Blizard,

It's a draft plan for the implementation of xembed plugin. Can you review it
and let me know if I am on the right way? Thank you very much!

Comment 19

[Christopher Blizzard (:blizzard)]

Looks good.  You will also need to handle key passing properly in the event that
the window in question doesn't have focus.

Comment 20

[Robin Lu]

Attachment: patch

Fixed the problem of flash crash (due to incorrect unrealize-destroy
procedure). Now java and flash plugin can work as good as before with this
patch.
Still not an XEmbeded patch.

Comment 21

[Robin Lu]

Attachment: skeletion for gtkxtbin supporting xembed

This is the primer gtkxtbin which support xembed. I didn't attach a patch
because it's like a new file. This depends on the last patch.

GtkSocket has to be added into a GtkContainer with proper GtkWidget hierachy
before it can work. I have tried two ways to introduce GtkSocket in:
1. Create a toplevel GtkWidget -> Create a GtkSocket -> Add the GtkSocket into
the toplevel GtkWidget -> Reparent the toplevel widget's GdkWindow to the port
window(inner_window) -> Create the Xt Widget and reparent it to the socket
id...
pro: an easy and clean way. 
con: break the hierachy. GtkSocket becomes dummy.
2. Let GtkXtBin inherit from GtkSocket -> Override realize(), use some hack way
to realize the GtkSocket without proper GtkWidget Hierachy -> Create the Xt
Widget and add it to the GtkXtBin...
pro: have chance to make GtkSocket work without a proper GtkWidget hierachy
con: need to rewrite part of GtkSocket code.

In the attached file, I choose the 2nd way. Current result is:
good ones:
1. flash works
2. can get XEMBED_WINDOW_ACTIVATE/DEACTIVATE
bad ones:
1. a temperary floating toplevel window can be seen
2. can not get focus related messages

The most reason for our GtkSocket dumb to the focus thing is that: although we
use gtk, we treat the focus in our own way in nsIWidget rather than that in
GtkContainer. In my opinion, if we want to support xembed in mozilla, we'd
better use a nsIWidget aware xembed than use GtkSocket directly. Even by using
GtkSocket, we have to write extra code in nsIWidget implemention to support
xembed focus.

My suggestion is to use the current stable patch(attach#91469). It works and
won't conflict much with the future enhancement for xembed. I think most xembed
code will be in gtkxtbin and nsWindow.

Comment 22

[Christopher Blizzard (:blizzard)]

You shouldn't have to do that at all.  The GtkSocket should be the direct child
of the container widget that all of Mozilla's windows are part of.  This is more
or less the same as how the scrollbar implementation works.  Look in
nsScrollbar::NativeCreate() for an example:

    // set the parent of the scrollbar to be the gdk window of the
    // parent window.
    gtk_widget_set_parent_window(mWidget, parentWindow);

    // add this widget to the parent window, assuming it's a container
    // of course.
    moz_container_put(MOZ_CONTAINER(parentWidget), mWidget,
                      mBounds.x, mBounds.y);

Comment 23

[Robin Lu]

Attachment: patch with xembed support

This is the plugin patch with support xembed for gtk2.
Two new files are added:gtk2xtbin.h and gtk2xtbin.c

Comment 24

[Robin Lu]

Attachment: another important new file

oops... I missed the defination file in the last patch(93132).
sorry for that.

Comment 25

[Robin Lu]

The current xembed patch can 
1. show java plugin and flash plugin currectly. 
2. catch xembed message and do response to XEMBED_FOCUS_IN/XEMBED_FOCUS_OUT if
GtkSocket would send them.
3. send XEMBED_REQUEST_FOCUS if plugin widget get FocusIn meesage.

There's one visible problem. Floating windows can be seen for less than a second
while plugin starts. I have another patch use the old core.window replacement
way (the other parts are the same) which does not have this problem.

Comment 26

[Christopher Blizzard (:blizzard)]

I'll have a look at this shortly.

There has to be a way to fix the floating window problem (the old code doesn't
have this problem.)  That's not acceptable and I don't want this checked in
until that's fixed.

Comment 27

[Robin Lu]

Attachment: patch with xembed support

This patch fixed the floating window.
seek r=?

ps. I reported a bug to gtk2 as:
http://bugzilla.gnome.org/show_bug.cgi?id=89496 [GtkSocket has no code to
remove its event filter]

Comment 28

[Robin Lu]

in patch for ns4xPluginInstance.cpp
#include "gtkxtbin.h" should be
#include "gtkx2xtbin.h"

Comment 29

[Christophe Fergeau]

In mozilla/widget/src/gtkxtbin/Makefile.in, 

+ifdef MOZ_ENABLE_GTK2
 EXPORTS         = gtkxtbin.h 
+else
+EXPORTS         = gtk2xtbin.h 
+endif

The two EXPORTS lines should exchanged, shouldn't they ?


+ifdef MOZ_ENABLE_GTK2
+CSRCS           = \
+                gtk2xtbin.c \
+
+else
 CSRCS           = \
                 gtkxtbin.c \
 
+endif

I also had to remove the \ at the end of each lines for this patch to compile.

Comment 30

[Robin Lu]

Fergeau,

Thanks for trying my patch.

For the first one, you are right. It's my mistake.

For the second one, I just followed the former way. Notise there's a blank line
after each gtk*xtbin.c\. Maybe it's not a compatible way. Could you tell me how
you compile it? I'll figure out if I should change them.

I do appraciate your comments!

Comment 31

[Robin Lu]

Attachment: patch support xembed

patch fixed the problem in makefile and head file.
The unparent problem is also fixed in this patch.
seek r=?

Comment 32

[Robin Lu]

The problem of the patch is that the plugin can not get focus at all.
I have tried a javaapplet with editorbox but can not type in it.
But since the java plugin doesn't use gtkxtbin at all (java plugin dose not call
the method SetWindow). It could not be the problem of xembed. So I tried the old
patch without xembed support and the result is the same - plugin can not get focus.

Comment 33

[Christopher Blizzard (:blizzard)]

Attachment: patch

patch with some minor build and formatting changes

Comment 34

[Robin Lu]

Attachment: patch

Fixed the focus problem of the plugins who use gtkxtbin ( not including java
plugin) by Blizzard's hacky proposal:) Maybe not that hacky, I used the xembed
way a little. Wish it is what you supposed.

Comment 35

[Robin Lu]

A url can be used for testing patch#94910
http://www.ajdigital.com/nfcchat/flash/

Comment 36

[Christopher Blizzard (:blizzard)]

Does this mean that java still isn't working?

Comment 37

[Robin Lu]

Attachment: patch

patch support focus.
For xembed plugin, the focus is get via XEMBED_FOCUS_REQUEST, for example,
flash plugin.
For non xembed plugin, the focus is get via enter/exit event, for example, java
plugin.

Comment 38

[Robin Lu]

reassign->[e-mail]

Comment 39

[Christopher Blizzard (:blizzard)]

Does this mean that we have both flash and java working?

Comment 40

[Robin Lu]

Blizzard,

Yes!

Comment 41

[Robin Lu]

Attachment: patch

Mostly same as attach 95987. fixed a bug in gtk2xtbin when query tree to
prevent memory leak.

Comment 42

[serge (gone)]

*** Bug 164792 has been marked as a duplicate of this bug. ***

Comment 43

[Robin Lu]

Attachment: patch after another round of reviewing

modified the error we met last time. Furthermore, I modified the code to also
XQuery when we get CreateNotify because when we get that event the window may
already have child.
Blizzard, I would very appreciate if you could review the patch by the end of
this weekend.

Comment 44

[Christopher Blizzard (:blizzard)]

OK, initial code checked in.  Thanks!

Comment 45

[Robin Lu]

Attachment: a missing patch for Makefile

Blizzard,

I noticed that you removed my change of the Makefile.in at /mozilla in
attachment 94577 [details] [diff] [review] . It works for linux and I forgot to double check the other
platform. However, My colleague just informed me that the build fails on
solaris. We should change the Makefile.in at /mozilla to make sure that the
gtkxtbin has been built before module/plugin.

Here it is the patch for Makefile.in at mozilla root. Sorry for that!

Comment 46

[Robin Lu]

need to change the /mozilla/Makefile.in

Comment 47

[Christopher Blizzard (:blizzard)]

Over to bug 166202.  Let's keep this closed and just fix bugs.

Comment 48

[redtux]

Is there a reason why /usr/lib/mozillaxx/plugins is not getting created or
libnullplugin.

Libnullplugin.so from a gtk1 build seems to work, but this does not seem right.

Also Realplayer8 reliably crashes mozilla

This is HEAD of Sunday am 29/9/2002

Comment 49

[shrirang khanzode]

.