191072 - [gtk2] make focus of non-xembed plugin work for moz+gtk2

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Robin Lu]

To compatible with exsiting non-xembed plugins and make the focus of these
plugins work.

Comment 1

[Robin Lu]

Attachment: patch

Comment 2

[Christopher Blizzard (:blizzard)]

Comment on attachment 112954 [details] [diff] [review]
patch

The patch here looks fine.  All my comments are regarding style.

>+    LOGFOCUS(("nsWindow::LoseNonXEmbedPluginFocus\n"));
>+    // This method is only for the nsWindow which contains a
>+    // Non-XEmbed plugin, for example, JAVA plugin.
>+    if(gPluginFocusWindow!=this) {
>+        return;
>+    }

This should be:

if (gPluginFocusWindow != this) {
    return;
}

note the use of whitespace.


>+    if(!curFocusWindow|| 
>+        curFocusWindow==GDK_WINDOW_XWINDOW(mDrawingarea->inner_window)){
>+        gdk_error_trap_push();

Also:

if (!curFocusWindow ||
     curFocusWindow == GDK_WINDOW_XWINDOW(mDrawingarea->inner_window)) {
     gdk_error_trap_push();
...

You should probably also add some veritcal whitespace here to try to break up
the lines.  Think of coding as sentences and paragraphs (there might be a
cultural difference here.)

I like this a lot more than the old code, fwiw. :)

Comment 3

[Robin Lu]

Attachment: patch

rearranged according to the comments and added some comments.
With this patch, we could seperate the treatments of xembed and non-xembed
plugins a little easier and cleaner in the future. It might be a long time for
that two kinds of plugins coexist. I know you are working at the xembed one so
I add nothing for it in my patch.

Comment 4

[Christopher Blizzard (:blizzard)]

There were still style problems in this patch, but I fixed them and checked it
in instead of wasting time and making us go around again.  Just FYI there were
still things like this:

if(foo) {

instead of:

if (foo) {

and some of this as well:

if (foo){

and even some of this:

if ( foo ) {

Comment 5

[Robin Lu]

Thanks!

Comment 6

[Robin Lu]

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

Comment 7

[Robin Lu]

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