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 514396 - Add gtk_show_uri
Add gtk_show_uri
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.12.x
Other All
: Normal enhancement
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 573964
 
 
Reported: 2008-02-04 20:04 UTC by Jaap A. Haitsma
Modified: 2009-04-28 13:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch adding gtk_show_uri and gtk_show_help to GTK+ (5.94 KB, patch)
2008-02-20 21:53 UTC, Jaap A. Haitsma
none Details | Review
Updated patch. Adding ret and gtk symbols (6.33 KB, patch)
2008-02-20 22:37 UTC, Jaap A. Haitsma
needs-work Details | Review
Patch removing gtk_show_help (4.88 KB, patch)
2008-02-29 22:31 UTC, Jaap A. Haitsma
none Details | Review
Updated gtk_show_uri patch now using a GdkScreen as input (4.83 KB, patch)
2008-03-23 23:12 UTC, Jaap A. Haitsma
none Details | Review
Updated patch. GdkScreen can be NULL (5.01 KB, patch)
2008-04-27 08:53 UTC, Jaap A. Haitsma
committed Details | Review

Description Jaap A. Haitsma 2008-02-04 20:04:59 UTC
Currently apps use gnome_help functions to show help files, but gnome_help is part of the deprecated gnome_help

See also GNOME goal proposal http://live.gnome.org/action/show/GnomeGoals/RemoveGnomeOpenGnomeHelp
Comment 1 Jaap A. Haitsma 2008-02-20 21:53:25 UTC
Created attachment 105668 [details] [review]
Patch adding gtk_show_uri and gtk_show_help to GTK+
Comment 2 Christian Persch 2008-02-20 22:06:32 UTC
The gtk_show_help function is missing |return ret;|.
You forgot to add the new funcs to gtk/gtk.symbols .

+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.

There is no "Lesser GPL version 2". It's the Lesser GPL version 2.1 (or the Library GPL version 2).
Comment 3 Sven Neumann 2008-02-20 22:29:50 UTC
Somehow I feel that gtk_show_help() is too GNOME oriented to be included in this form. I have been told that gtk_show_uri() will work nicely on the major supported platforms. This is hwoever not the case for gtk_show_help(). Since the functionality of gtk_show_help() can be easily implemented on top of gtk_show_uri(), it should be sufficient to provide gtk_show_uri() for now.
Comment 4 Jaap A. Haitsma 2008-02-20 22:37:42 UTC
Created attachment 105671 [details] [review]
Updated patch. Adding ret and gtk symbols

Christian, I copied the license from other files in GTK+. I opened about 5 to 6 files and they are the same. I guess it's a better idea to change all licenses in one go
Comment 5 Jaap A. Haitsma 2008-02-23 17:33:56 UTC
What's the consensus on this?

Can I commit gtkshow.c to gtk+ when I remove gtk_show_help? 
Comment 6 Bastien Nocera 2008-02-29 21:35:35 UTC
Update your patch to remove gtk_show_help() to start with.
Comment 7 Jaap A. Haitsma 2008-02-29 22:31:44 UTC
Created attachment 106302 [details] [review]
Patch removing gtk_show_help
Comment 8 Jaap A. Haitsma 2008-03-07 21:06:05 UTC
Can one of the maintainers decided what to do with this patch? Thanks
Comment 9 Morten Welinder 2008-03-16 21:03:09 UTC
gtk_show_uri allows a NULL parent, but that is not multi-head safe.
Why not take a mandatory GdkScreen argument since the function couldn't
care less about the parent widget beyond its screen?
Comment 10 Jaap A. Haitsma 2008-03-16 21:44:27 UTC
I can make it such that the parent has to be set mandatory.

Having a GtkWidget *parent saves calling a gtk_widget_get_screen everytime you do a gtk_show_uri

What's the opinion on this of others?
Comment 11 Ross Burton 2008-03-16 22:22:12 UTC
I don't think the parent should be mandatory, because there are situations where they may not be a parent at all.
Comment 12 Sven Neumann 2008-03-18 16:08:00 UTC
A mandatory parent would be bad, as Ross explained in comment #11. IMO the best solution is to change the function to take a GdkScreen instead of a parent widget. Shouldn't hurt much to call gtk_widget_get_screen().
Comment 13 Jaap A. Haitsma 2008-03-23 23:12:30 UTC
Created attachment 107888 [details] [review]
Updated gtk_show_uri patch now using a GdkScreen as input

Thanks for you comments. Any more comments?
Comment 14 Jaap A. Haitsma 2008-04-04 05:54:02 UTC
Ping. Can above patch be applied?
Comment 15 Jaap A. Haitsma 2008-04-19 19:07:00 UTC
Can one of the maintainers make a decision about this patch.

Thanks
Comment 16 Matthias Clasen 2008-04-26 22:14:56 UTC
Some comments: 

- Wrt to the parent issue, I think the function should accept NULL there. 
  Being forced to call gdk_widget_get_screen() is annoyance, when 
  GdkAppLaunchContext handles unset screen already.

- The documentation needs to go in some more detail about what kind of 'uri' this function accepts. There is no standard for this, unfortunately, and the gnome-vfs->gio transition has unearthed a number of problems due to this.

- I'd be interested in a function that mounts the volume before opening it. This is what e.g. the panel needs (see bug 529243 ). That could be a separate function, or gtk_uri_show could take flags.
Comment 17 Jaap A. Haitsma 2008-04-27 08:53:40 UTC
Created attachment 109974 [details] [review]
Updated patch. GdkScreen can be NULL

Matthias, thanks for the review. 

I've updated the patch with your first two comments.

About the third comment (the auto mounting). Why would gtk_show_uri need extra flags for this? We could add code that just tries to mount if this is necessary. Your patch in bug 529243 uses libgnomeui/gnome-password-dialog.h which can't be used in gtk+

What do you want to do? Commit this patch (quite some apps could reduce their dependency on libgnomeui and friends already) and later on add the mounting part
Comment 18 Morten Welinder 2008-04-27 14:04:04 UTC
Matthias: being forced to call gdk_widget_get_screen is a good thing as
it avoids multiscreen bugs.  If GdkAppLaunchContext handles it, as you
say, that sounds like that ought to be fixed too.  There really is no
way the gdk/gtk layer can know what screen should be used.
Comment 19 Dan Winship 2008-04-27 16:09:36 UTC
OK, this bug report has gotten silly.</monty-python>

In 99% of use cases, the user interacts with some widget, and as a result, the program wants to show a URI on the same screen as that widget.

In 99% of the remaining 1% of use cases, the program has no opinion at all about what screen to show the URI on (eg, gnome-open), and so it's going to pass NULL for parent regardless of whether parent is a GtkWidget or a GtkScreen. (Or in the mandatory GtkScreen case, it would always call gdk_screen_get_default().)

Given that this is a 10-line function, it seems silly to make its interface less convenient for 99% of users just so it can cover an additional 0.01% of use cases. Assuming there actually are programs that need to show URIs on specific screens that do not correspond to any live widget, the developers of those programs can just cut and paste the gtk_show_uri source and change it accordingly.
Comment 20 Matthias Clasen 2008-05-25 03:41:11 UTC
Jaap: the way to handle mounting in GTK+ is to use a GtkMountOperation. 

I'm not 100% happy with the single-function header gtkshow.h, but I don't really
have a better alternative.

It is probably ok to commit it in this form now, we'll figure out the mounting later.  
Comment 21 Jaap A. Haitsma 2008-05-25 08:51:41 UTC
Thanks for the review. It's now committed

2008-05-25  Jaap A. Haitsma  <jaap@haitsma.org>

	reviewed by: Matthias Clasen

	* gtk/Makefile.am:
	* gtk/gtk.h:
	* gtk/gtk.symbols:
	* gtk/gtkshow.c: (gtk_show_uri):
	* gtk/gtkshow.h:
	Add gtk_show_uri to make showing uris really easy. Fixes bug #514396