After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642186 - xulrunner API: strict mode support changed some callbacks
xulrunner API: strict mode support changed some callbacks
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal blocker
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-12 18:34 UTC by Quentin "Sardem FF7" Glidic
Modified: 2011-02-16 16:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Conditionally adapt to new strict setters (9.55 KB, patch)
2011-02-12 19:23 UTC, Marc-Antoine Perennou
none Details | Review
Conditionally adapt to new strict setters (8.86 KB, patch)
2011-02-15 16:37 UTC, Marc-Antoine Perennou
reviewed Details | Review
Just an example to avoid the warning (1.54 KB, patch)
2011-02-15 17:01 UTC, Marc-Antoine Perennou
none Details | Review
Silent a warning (773 bytes, patch)
2011-02-15 17:21 UTC, Marc-Antoine Perennou
none Details | Review
Silent a warning with JSLocaleToUnicode (2.76 KB, patch)
2011-02-15 19:23 UTC, Marc-Antoine Perennou
none Details | Review
Silent a warning with JSLocaleToUnicode (2.77 KB, patch)
2011-02-15 19:35 UTC, Marc-Antoine Perennou
reviewed Details | Review
Silence a warning with JSLocaleToUnicode (2.75 KB, patch)
2011-02-16 16:39 UTC, Marc-Antoine Perennou
none Details | Review

Description Quentin "Sardem FF7" Glidic 2011-02-12 18:34:07 UTC
They are implementing the strict mode support, which needs some strict mode aware callbacks.
One is the property set one, which is used in GJS.

Mozilla Mercurial changeset:
http://hg.mozilla.org/mozilla-central/rev/4b56bfdf61a7
Comment 1 Marc-Antoine Perennou 2011-02-12 18:54:34 UTC
Working on it, still the conditionnal check to do and some warning to shut up and it's ok
Comment 2 Marc-Antoine Perennou 2011-02-12 19:23:46 UTC
Created attachment 180733 [details] [review]
Conditionally adapt to new strict setters

This patch allows all gjs tests to pass with xulrunner from hg
Comment 3 Marc-Antoine Perennou 2011-02-15 16:37:47 UTC
Created attachment 180913 [details] [review]
Conditionally adapt to new strict setters

Simplify patch
Comment 4 Marc-Antoine Perennou 2011-02-15 17:01:18 UTC
Created attachment 180922 [details] [review]
Just an example to avoid the warning

Btw, I didn't find the commit which involved that, but I think it's linked :
JSLocaleCallbacks now needs a JSErrorCallback
Here is a dummy patch just to show the structure of it, no implementation
Comment 5 Marc-Antoine Perennou 2011-02-15 17:21:05 UTC
Created attachment 180928 [details] [review]
Silent a warning

Ok, actually this fifth element has been there for some time now, the warning was not cuased by this missing, but because of an argument of gjs_locale_to_unicode being a char instead of a const char
Comment 6 Marc-Antoine Perennou 2011-02-15 17:25:22 UTC
Here is the commit (dunno how to dynamically check for this one, though)
http://hg.mozilla.org/mozilla-central/rev/f971ad6ed5a5
Comment 7 Christopher Aillon 2011-02-15 18:00:07 UTC
(In reply to comment #6)
> Here is the commit (dunno how to dynamically check for this one, though)
> http://hg.mozilla.org/mozilla-central/rev/f971ad6ed5a5

You can do something like:

AC_MSG_CHECKING([whether JSLocaleToUnicode takes a const char*])
_SAVED_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS	-Werror"
AC_TRY_COMPILE([#include "jspubtd.h"],
               [...], 
               [ac_cv_JSLocaleToUnicode_const_char=1 AC_MSG_RESULT([yes])],
               [ac_cv_JSLocaleToUnicode_const_char=0 AC_MSG_RESULT([no])])
CFLAGS="$_SAVED_CFLAGS"
Comment 8 Marc-Antoine Perennou 2011-02-15 19:23:18 UTC
Created attachment 180939 [details] [review]
Silent a warning with JSLocaleToUnicode

Thx, done that in a slightly different way, but with the same concept
Comment 9 Marc-Antoine Perennou 2011-02-15 19:35:08 UTC
Created attachment 180940 [details] [review]
Silent a warning with JSLocaleToUnicode

Copy-paste fail, sorry
Comment 10 Colin Walters 2011-02-16 16:34:22 UTC
Review of attachment 180913 [details] [review]:

This won't actually compile on older xulrunner, will it?  There's no JSStringPropertyStub there.  We may have to add a wrapper in jsapi-util.h?
Comment 11 Colin Walters 2011-02-16 16:35:59 UTC
Review of attachment 180940 [details] [review]:

"Silent" should be "Silence" in your commit message.

::: configure.ac
@@ +214,3 @@
+    )],
+    [ac_cv_JSLocaleToUnicode_const_char=yes],
+    [ac_cv_JSLocaleToUnicode_const_char=no])

I am confused why you're using "ac_cv_" prefix; those are supposed to be "private" to autoconf, right?  Why not name the variable have_jslocale_to_unicode_const say?
Comment 12 Marc-Antoine Perennou 2011-02-16 16:36:38 UTC
(In reply to comment #10)
> Review of attachment 180913 [details] [review]:
> 
> This won't actually compile on older xulrunner, will it?  There's no
> JSStringPropertyStub there.  We may have to add a wrapper in jsapi-util.h?

It will, I think, see :

diff --git a/gjs/compat.h b/gjs/compat.h
index d418a22..09c2b98 100644
--- a/gjs/compat.h
+++ b/gjs/compat.h
@@ -135,6 +135,10 @@ gjs_##name##_constructor(JSContext *context,            \
 
 #endif
 
+#ifndef HAVE_JS_STRICTPROPERTYSTUB
+#define JS_StrictPropertyStub JS_PropertyStub
+#endif
+
 G_END_DECLS
Comment 13 Marc-Antoine Perennou 2011-02-16 16:39:30 UTC
Created attachment 181020 [details] [review]
Silence a warning with JSLocaleToUnicode

Renamed (didn't know how to name it so just took the name from the comment above)
Comment 14 Colin Walters 2011-02-16 16:40:14 UTC
(In reply to comment #12)

> +#ifndef HAVE_JS_STRICTPROPERTYSTUB
> +#define JS_StrictPropertyStub JS_PropertyStub
> +#endif
> +
>  G_END_DECLS

Ah, I missed that.  Yes, I can confirm we still compile on xulrunner 1.9.2 from Fedora 14.
Comment 15 Colin Walters 2011-02-16 16:41:14 UTC
Pushed both patches, thank you!