Closed Bug 297238 Opened 19 years ago Closed 19 years ago

incorrect parameter specification for signal open-uri of gtkembedmoz

Categories

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

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gjc, Assigned: chpe)

Details

(Keywords: fixed1.8)

Attachments

(1 file, 2 obsolete files)

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:
PS: the guy mentions debian unstable, so I figure this is version 1.7.8-1.
Assignee: general → mpgritti
Component: General → Embedding: GTK Widget
Product: Mozilla Application Suite → Core
QA Contact: general → pavlov
Version: unspecified → 1.7 Branch
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?
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.
Attached patch fix (obsolete) — Splinter Review
Use the right value types AND the right marshalers.

gtkmozembedmarshal.[ch] are auto-generated from the changed types.txt with
glib-genmarshal.
Attachment #185831 - Flags: superreview?(blizzard)
Attachment #185831 - Flags: review?(mpgritti)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 1.7 Branch → Trunk
Comment on attachment 185831 [details] [diff] [review]
fix

Why arent we fixing gtk1 too?
Attached patch updated patch (obsolete) — Splinter Review
Try to fix gtk1 too. Apparently on gtk1 we can use the POINTER marshals for
STRING type too.
Attachment #185831 - Attachment is obsolete: true
Attachment #185851 - Flags: superreview?(blizzard)
Attachment #185851 - Flags: review?(mpgritti)
Attachment #185831 - Flags: superreview?(blizzard)
Attachment #185831 - Flags: review?(mpgritti)
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.
Attachment #185851 - Flags: review?(mpgritti) → review+
the patch compiles fine for me on gtk1, and TestGtkEmbed still shows:
open_uri_cb http://www.mozilla.org/
Has this been committed to some branch?
Attachment #185851 - Flags: superreview?(blizzard) → superreview?(roc)
Attachment #185851 - Flags: superreview?(roc) → superreview+
Comment on attachment 185851 [details] [diff] [review]
updated patch

Embed-only fix, no risk, important for language bindings.
Attachment #185851 - Flags: approval1.8b4?
Attachment #185851 - Flags: approval1.8b4? → approval1.8b4+
line break as requested in comment 7
Attachment #185851 - Attachment is obsolete: true
Assignee: mpgritti → chpe
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
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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: