297238 - incorrect parameter specification for signal open-uri of gtkembedmoz

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GTK Widget, defect)

Description

[Gustavo J. A. M. Carneiro]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050512 Epiphany/1.6.1 (Ubuntu) (Ubuntu package 1.0.2)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050512 Epiphany/1.6.1 (Ubuntu) (Ubuntu package 1.0.2)

See http://bugzilla.gnome.org/show_bug.cgi?id=307057

Basically the 'url' parameter of the 'open-uri' signal of gtkembedmoz is being
incorrectly registerd as a 'pointer' when it should be 'string'.  It makes no
difference in C apps, but for Python bindings it makes a _lot_ of difference. 
Please fix.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Gustavo J. A. M. Carneiro]

PS: the guy mentions debian unstable, so I figure this is version 1.7.8-1.

Comment 2

[Christian Persch (GNOME) (away; not receiving bug mail)]

Bug 58325 was about the same thing, had a patch, which is marked 'checked in',
but it's not!

IMHO the patch in bug 58325 was wrong, though: it changed only the value types,
but didn't use the right marshaler. We need to generate new marshalers for the
STRING types, but glib-genmarshal produces glib2 code... any idea how to do this?

Comment 3

[Christian Persch (GNOME) (away; not receiving bug mail)]

CVS tells me the patch from bug 58325 caused bug 183912; there they changed it
back to POINTER instead of fixing the bug, which is the wrong marshalers.

Comment 4

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: fix

Use the right value types AND the right marshalers.

gtkmozembedmarshal.[ch] are auto-generated from the changed types.txt with
glib-genmarshal.

Comment 5

[Marco Pesenti Gritti]

Comment on attachment 185831 [details] [diff] [review]
fix

Why arent we fixing gtk1 too?

Comment 6

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch

Try to fix gtk1 too. Apparently on gtk1 we can use the POINTER marshals for
STRING type too.

Comment 7

[Marco Pesenti Gritti]

Comment on attachment 185851 [details] [diff] [review]
updated patch

Looks good to me. Though test on gtk1 would be appreciated.

+		   GTK_TYPE_BOOL, 1, GTK_TYPE_STRING |
G_SIGNAL_TYPE_STATIC_SCOPE);

Prolly want to break line  like above here. We can change that when committing.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

the patch compiles fine for me on gtk1, and TestGtkEmbed still shows:
open_uri_cb http://www.mozilla.org/

Comment 9

[Gustavo J. A. M. Carneiro]

Has this been committed to some branch?

Comment 10

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 185851 [details] [diff] [review]
updated patch

Embed-only fix, no risk, important for language bindings.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch for checkin

line break as requested in comment 7

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

HEAD:
Checking in embedding/browser/gtk/src/EmbedProgress.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedProgress.cpp,v  <-- 
EmbedProgress.cpp
new revision: 1.16; previous revision: 1.15
done
Checking in embedding/browser/gtk/src/gtkmozembed2.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v  <--  gtkmozembed2.cpp
new revision: 1.45; previous revision: 1.44
done
Checking in embedding/browser/gtk/src/gtkmozembedmarshal.c;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembedmarshal.c,v  <-- 
gtkmozembedmarshal.c
new revision: 1.2; previous revision: 1.1
done
Checking in embedding/browser/gtk/src/gtkmozembedmarshal.h;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembedmarshal.h,v  <-- 
gtkmozembedmarshal.h
new revision: 1.2; previous revision: 1.1
done
Checking in embedding/browser/gtk/src/types.txt;
/cvsroot/mozilla/embedding/browser/gtk/src/types.txt,v  <--  types.txt
new revision: 1.2; previous revision: 1.1
done

MOZILLA_1_8_BRANCH:
Checking in embedding/browser/gtk/src/EmbedProgress.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedProgress.cpp,v  <-- 
EmbedProgress.cpp
new revision: 1.15.20.1; previous revision: 1.15
done
Checking in embedding/browser/gtk/src/gtkmozembed2.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v  <--  gtkmozembed2.cpp
new revision: 1.43.8.2; previous revision: 1.43.8.1
done
Checking in embedding/browser/gtk/src/gtkmozembedmarshal.c;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembedmarshal.c,v  <-- 
gtkmozembedmarshal.c
new revision: 1.1.92.1; previous revision: 1.1
done
Checking in embedding/browser/gtk/src/gtkmozembedmarshal.h;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembedmarshal.h,v  <-- 
gtkmozembedmarshal.h
new revision: 1.1.92.1; previous revision: 1.1
done
Checking in embedding/browser/gtk/src/types.txt;
/cvsroot/mozilla/embedding/browser/gtk/src/types.txt,v  <--  types.txt
new revision: 1.1.92.1; previous revision: 1.1
done