Bug 25263

Summary:

[Gtk] WebKit GTK with libsoup won't recognize proxies

Product:

WebKit

Reporter:

pzk5qt6ottv3z7ag

Component:

WebKitGTK

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

christian, gustavo, jmalonzo, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
patch for enable proxy in libsoup	none
to enable proxy-resolver feature in libsoup-gnome	jmalonzo: review-
enable libsoup-gnome features, default=no	none
added an option to enable GNOME-specific features of libsoup, default=no	none
added an option to enable GNOME-specific features of libsoup, default=no	jmalonzo: review-
added an option to enable GNOME-specific features of libsoup, default=no	none

pzk5qt6ottv3z7ag

Reported 2009-04-17 02:07:48 PDT

Environment: Debian 2.6.18 amd64 latest libsoup & libproxy till now WebKit 1.1.5 (should also affect latest nightly build) My computer connect with Internet through a HTTP proxy, which is set in the 'http_proxy' environment variable. I've been working on WebKit for months, at first, while I was using 1.0.1, (with curl as its backend), everything's all right. But recently I updated it to 1.1.5, and found that GtkLauncher could not load http://www.google.com. It took me some time to realize that the network backend has been changed to libsoup. And I did test my libsoup, which worked well. It took me another some time to compare WebCore/platform/network/soup/ResourceHandlSoup.cpp and test.c in libsoup tests. And at last my conclusion is maybe the proxy-resolver feature is not enabled. So I made a dirty patch: 1.in configure, add dependency rule of libsoup-gnome-2.4 (I failed in editing configure.ac and re-automake, for some unknown reason) 2.in WebCore/platform/network/soup/ResourceHandleSoup.cpp 1) at the beginning, add #include <libsoup/soup-gnome.h> 2) at arount line 444, replace the body of function 'static SoupSession* createSoupSession()' with the following code return soup_session_async_new_with_options(SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_GNOME_FEATURES_2_26, NULL); I don't know whether it'll affect other parts of webkitgtk, but at least it works for me so please check this, thx

Attachments
patch for enable proxy in libsoup (671 bytes, patch) 2009-04-17 02:16 PDT, pzk5qt6ottv3z7ag	no flags	Details Formatted Diff Diff
to enable proxy-resolver feature in libsoup-gnome (3.48 KB, patch) 2009-04-20 00:26 PDT, pzk5qt6ottv3z7ag	jmalonzo: review-	Details Formatted Diff Diff
enable libsoup-gnome features, default=no (4.32 KB, patch) 2009-05-21 01:40 PDT, pzk5qt6ottv3z7ag	no flags	Details Formatted Diff Diff
added an option to enable GNOME-specific features of libsoup, default=no (4.30 KB, patch) 2009-05-21 01:53 PDT, pzk5qt6ottv3z7ag	no flags	Details Formatted Diff Diff
added an option to enable GNOME-specific features of libsoup, default=no (4.31 KB, patch) 2009-05-21 02:28 PDT, pzk5qt6ottv3z7ag	jmalonzo: review-	Details Formatted Diff Diff
added an option to enable GNOME-specific features of libsoup, default=no (4.76 KB, patch) 2009-05-30 23:23 PDT, pzk5qt6ottv3z7ag	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

pzk5qt6ottv3z7ag

Comment 1 2009-04-17 02:16:24 PDT

Created attachment 29573 [details] patch for enable proxy in libsoup based on Webkit-r42569

Jan Alonzo

Comment 2 2009-04-19 02:51:35 PDT

(In reply to comment #1) > Created an attachment (id=29573) [review] > patch for enable proxy in libsoup > > based on Webkit-r42569 > This is good. For adding the configure option, you could just look at the LIBSOUP parts in configure.ac and add a LIBSOUP_GNOME and check for libsoup-gnome-2.4. Please add a ChangeLog and set the review flag to "?" as well so reviewers can review your changes. Thanks,

pzk5qt6ottv3z7ag

Comment 3 2009-04-20 00:26:20 PDT

Created attachment 29611 [details] to enable proxy-resolver feature in libsoup-gnome Detect libsoup-gnome, and use it instead of libsoup, if exists. If libsoup-gnome exists, enable SOUP_TYPE_GNOME_FEATURES_2_26 when creating a new soup session

Xan Lopez

Comment 4 2009-04-20 06:47:49 PDT

I'm really not sure that this should be done in webkit, or that it should autodetect the library and enable the proxy without explicit setup (we are not doing this for anything that it's not a hard dependency AFAIK).

Jan Alonzo

Comment 5 2009-05-15 17:15:51 PDT

Comment on attachment 29611 [details] to enable proxy-resolver feature in libsoup-gnome (In reply to comment #3) > Created an attachment (id=29611) [review] > to enable proxy-resolver feature in libsoup-gnome > > Detect libsoup-gnome, and use it instead of libsoup, if exists. > If libsoup-gnome exists, enable SOUP_TYPE_GNOME_FEATURES_2_26 when creating a > new soup session Since libsoup-gnome is not a hard-dependency, this feature should be configurable at configure time and should be disabled by default. I would suggest adding --with-gnome or --with-libsoup-gnome in configure.ac for this. I'm going to say r- for now until have this feature configurable.

pzk5qt6ottv3z7ag

Comment 6 2009-05-21 01:40:52 PDT

Created attachment 30529 [details] enable libsoup-gnome features, default=no this is based on r43933 because I can't build latest svn version, see https://bugs.webkit.org/show_bug.cgi?id=25910

pzk5qt6ottv3z7ag

Comment 7 2009-05-21 01:53:44 PDT

Created attachment 30531 [details] added an option to enable GNOME-specific features of libsoup, default=no this is based on r43961, because latest svn version of webkitgtk could not compile, see https://bugs.webkit.org/show_bug.cgi?id=25910

pzk5qt6ottv3z7ag

Comment 8 2009-05-21 02:28:39 PDT

Created attachment 30535 [details] added an option to enable GNOME-specific features of libsoup, default=no this is based on r43961, since I couldn't compile the lastest svn version of webkitgtk, see https://bugs.webkit.org/show_bug.cgi?id=25910 besides, sorry for the last 2 failed patches, this one should work

Jan Alonzo

Comment 9 2009-05-22 20:40:23 PDT

Comment on attachment 30535 [details] added an option to enable GNOME-specific features of libsoup, default=no > Index: ChangeLog > =================================================================== > --- ChangeLog (revision 43961) > +++ ChangeLog (working copy) > @@ -1,3 +1,12 @@ > +2009-05-21 WANG Lu <coolwanglu@gmail.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Added --with-libsoup-gnome (default=no) to enable GNOME-specific > + features of libsoup, an important one is the proxy-resovler feature Please fix the spaces here. > Index: configure.ac > =================================================================== > --- configure.ac (revision 43933) > +++ configure.ac (working copy) > @@ -497,7 +497,6 @@ AC_ARG_ENABLE([jit], > if test "$enable_jit" = "yes"; then > case "$host_cpu" in > i*86|x86_64) > - AC_DEFINE([ENABLE_JIT], [1], [Define to enable JIT]) Is this part of the patch? > AC_DEFINE([ENABLE_YARR], [1], [Define to enable YARR]) > AC_DEFINE([ENABLE_YARR_JIT], [1], [Define to enable YARR JIT]) > AC_DEFINE([ENABLE_JIT_OPTIMIZE_CALL], [1], [Define to enable optimizing calls]) > @@ -550,10 +549,28 @@ else > CFLAGS="$CFLAGS -O0" > fi > > -PKG_CHECK_MODULES([LIBSOUP], > - [libsoup-2.4 >= $LIBSOUP_REQUIRED_VERSION]) > -AC_SUBST([LIBSOUP_CFLAGS]) > -AC_SUBST([LIBSOUP_LIBS]) > +# check whether to enable libsoup-gnome > +AC_MSG_CHECKING([whether to use GNOME-specific features of libsoup]) > +AC_ARG_WITH(libsoup_gnome, > + AC_HELP_STRING([--with-libsoup-gnome], > + [using GNOME-specific features of libsoup [default=no]]), > + [],[with_libsoup_gnome="no"]) > +AC_MSG_RESULT([$with_libsoup_gnome]) > + > +if test "$with_libsoup_gnome" = "yes"; then > + PKG_CHECK_MODULES([LIBSOUP], > + [libsoup-gnome-2.4 >= $LIBSOUP_REQUIRED_VERSION]) # FIXME: does libsoup-gnome need a separated REQUIRED_VERSION ? FIXME is not required. They should be the same. > + AC_DEFINE([ENABLE_LIBSOUP_GNOME],[1],[Define to enable libsoup-gnome]) Please use USE_SOUP_GNOME and this should be AM_CONDITIONAL. And move this in the end of this file. > + AC_SUBST(LIBSOUPE_CFLAGS) > + AC_SUBST(LIBSOUPE_LIBS) LIBSOUPE? > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 43961) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,13 @@ > +2009-05-21 WANG Lu <coolwanglu@gmail.com> > + > + Reviewed by NOBODY (OOPS!). > + > + If libsoup-gnome is enabled, add proper parameters to enable > + GNOME-specific features while creating a new soup session. Fix the indentation please. > Index: WebCore/platform/network/soup/ResourceHandleSoup.cpp > =================================================================== > --- WebCore/platform/network/soup/ResourceHandleSoup.cpp (revision 43933) > +++ WebCore/platform/network/soup/ResourceHandleSoup.cpp (working copy) > @@ -48,7 +48,12 @@ > #include <fcntl.h> > #include <gio/gio.h> > #include <gtk/gtk.h> > + > +#ifdef ENABLE_LIBSOUP_GNOME Should be USE_SOUP_GNOME per comment above. Then USE(SOUP_GNOME) here. > #include <libsoup/soup.h> > + Not required. > static SoupSession* createSoupSession() > { > +#ifdef ENABLE_LIBSOUP_GNOME USE(SOUP_GNOME) here. > + return soup_session_async_new_with_options(SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_GNOME_FEATURES_2_26, > + NULL); And put this all in one line please. r- for now until the comments above have been addressed. Thanks!

pzk5qt6ottv3z7ag

Comment 10 2009-05-30 23:23:29 PDT

Created attachment 30810 [details] added an option to enable GNOME-specific features of libsoup, default=no

Jan Alonzo

Comment 11 2009-06-05 20:32:18 PDT

Comment on attachment 30810 [details] added an option to enable GNOME-specific features of libsoup, default=no r=me

Jan Alonzo

Comment 12 2009-06-05 22:20:59 PDT

Landed in r44479.

pzk5qt6ottv3z7ag

Comment 13 2009-06-06 00:44:32 PDT

Thanks a lot! (In reply to comment #12) > Landed in r44479. >

Xan Lopez

Comment 14 2009-06-07 10:54:02 PDT

I think this patch should be reverted, please see bug #26242 for rationale.

Jan Alonzo

Comment 15 2009-06-07 15:55:37 PDT

(In reply to comment #14) > I think this patch should be reverted, please see bug #26242 for rationale. This currently defaults to 'no' so only people who want it can have it. Let's discuss better ways to handle optional features in the future in bug #26242, and fix whatever is needed to be fixed in that bug. I rather have people behind a proxy can still browse the web using webkitgtk today instead of waiting for a concensus on #26242 (we may hit a road block and decide to recommit this patch again, who knows). Can I close this bug again and track whatever needs to be fixed in bug #26242?

Xan Lopez

Comment 16 2009-06-07 22:57:47 PDT

(In reply to comment #15) > (In reply to comment #14) > > I think this patch should be reverted, please see bug #26242 for rationale. > > This currently defaults to 'no' so only people who want it can have it. Let's > discuss better ways to handle optional features in the future in bug #26242, > and fix whatever is needed to be fixed in that bug. I rather have people behind > a proxy can still browse the web using webkitgtk today instead of waiting for a > concensus on #26242 (we may hit a road block and decide to recommit this patch > again, who knows). I think this is ignoring the main point of the issue: the 'people' we care about here are distros, and they will have to make a choice if we make a release with this option. That will only add a new variable to the equation and make things worse, when it seems that we already have a consensus that what we are doing is wrong. Besides, I believe this feature request in particular is slighltly artificial. I don't think GtkLauncher is a good use case for almost anything, and the WebKit apps that provide the hability to navigate the Web can trivially use the GNOME proxy settings if they want to (Epiphany is, for example). So it's not like reverting this patch will prevent people from using proxies, it will only put us back where we were before, when they have to manually request it in their applications.

Jan Alonzo

Comment 17 2009-06-07 23:30:53 PDT

(In reply to comment #16) > > I think this is ignoring the main point of the issue: the 'people' we care > about here are distros, and they will have to make a choice if we make a > release with this option. That will only add a new variable to the equation and > make things worse, when it seems that we already have a consensus that what we > are doing is wrong. Honestly, I wasn't aware of any concensus. I was assuming you're all OK with this since there were no comments since the patched was marked as r? > > Besides, I believe this feature request in particular is slighltly artificial. > I don't think GtkLauncher is a good use case for almost anything, and the > WebKit apps that provide the hability to navigate the Web can trivially use the > GNOME proxy settings if they want to (Epiphany is, for example). So it's not How about mobile platforms that want to use WebKitGtk? Or users that don't want GNOME but want to use WebKitGtk?

Xan Lopez

Comment 18 2009-06-07 23:35:20 PDT

(In reply to comment #17) > > Besides, I believe this feature request in particular is slighltly artificial. > > I don't think GtkLauncher is a good use case for almost anything, and the > > WebKit apps that provide the hability to navigate the Web can trivially use the > > GNOME proxy settings if they want to (Epiphany is, for example). So it's not > > How about mobile platforms that want to use WebKitGtk? Or users that don't want > GNOME but want to use WebKitGtk? > I'm not sure I get what you mean. If they don't want to use GNOME I guess they won't have much use for a --with-libsoup-gnome option, no matter if it's in WebKit or elsewhere. The only thing I'm saying is that whoever was interested in this configure option in WebKit can trivially replicate the functionality in his own application, like Epiphany is doing. If you are not interested you don't have to do anything.

Jan Alonzo

Comment 19 2009-06-07 23:40:23 PDT

Comment on attachment 30810 [details] added an option to enable GNOME-specific features of libsoup, default=no Clearing r+, setting it back to r?. Patch reverted in r44494 as suggested in this bug.

Xan Lopez

Comment 20 2009-06-10 12:09:13 PDT

Comment on attachment 30810 [details] added an option to enable GNOME-specific features of libsoup, default=no I think we can clear the review flag here for now?

Christian Dywan

Comment 21 2009-08-13 16:03:47 PDT

I think this is overkill. Enabling proxy server support or all GNOME features is a one-liner per GNOME application. And in my opinion, libsoup should have a SOUP_TYPE_PROXY that uses whatever is available without requiring everything at build time. Or something that takes a string argument.

Christian Dywan

Comment 22 2009-12-20 02:42:42 PST

Concluding it is a one-liner to add the GNOME proxy resolver to GNOME applications, we shouldn't do that in WebKitGTK+. And http_proxy can be handled in libsoup, I filed https://bugzilla.gnome.org/show_bug.cgi?id=605048 for that. So this is resolved as far as WebKitGTK+ is concerned.

Note You need to log in before you can comment on or make changes to this bug.