Last Comment Bug 396876 - GTK file picker doesn't show preview of file
: GTK file picker doesn't show preview of file
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- enhancement (vote)
: mozilla1.9beta3
Assigned To: Michael Ventnor
Depends on:
  Show dependency treegraph
Reported: 2007-09-20 04:31 PDT by versgui
Modified: 2007-12-19 03:30 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (6.92 KB, patch)
2007-12-16 22:04 PST, Michael Ventnor
no flags Details | Diff | Splinter Review
Patch 1.1 (6.92 KB, patch)
2007-12-16 22:06 PST, Michael Ventnor
no flags Details | Diff | Splinter Review
Patch 2 (6.62 KB, patch)
2007-12-17 11:48 PST, Michael Ventnor
no flags Details | Diff | Splinter Review
Patch 3 (7.67 KB, patch)
2007-12-18 13:27 PST, Michael Ventnor
roc: review+
roc: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review

Description versgui 2007-09-20 04:31:26 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv: Gecko/20070725 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; fr; rv: Gecko/20070725 Firefox/

(sorry for my bad english ^^)

When I open a file with File > Open or in a web page, I have this :

But it would be well better if one could see the contents of the files, like this:

It would seem that bad GTK widget is called, according to certain forums.

Reproducible: Always

Steps to Reproduce:
1. Open a file !
Comment 1 Jesse Ruderman 2007-09-21 00:02:40 PDT
If you want this feature added to the GTK file picker, you should probably contact the people who maintain the GTK file picker, not the people who maintain applications that use the GTK file picker.
Comment 2 versgui 2007-09-21 04:15:06 PDT
According to some other people of, the problem would come from Firefox which calls the bad widget : this widget already exists.
Comment 3 Christian Persch (GNOME) (away; not receiving bug mail) 2007-09-21 05:00:32 PDT
It's up to the application to provide a preview in the gtk filechooser (see gtk_file_chooser_set_preview_widget docs).
Comment 4 Michael Ventnor 2007-12-16 22:01:08 PST
Assigning to self as I have something that works reasonably well.
Comment 5 Michael Ventnor 2007-12-16 22:04:30 PST
Created attachment 293466 [details] [diff] [review]

Uses a GtkImage to display a preview of any image that is selected. I think sites like Photobucket and Flickr have made the need for something like this much higher ;)
Comment 6 Michael Ventnor 2007-12-16 22:06:20 PST
Created attachment 293467 [details] [diff] [review]
Patch 1.1

Damn the tabs.
Comment 7 Christian Persch (GNOME) (away; not receiving bug mail) 2007-12-17 05:31:11 PST
+  if (img_preview)
+    gtk_widget_destroy(img_preview);

I don't think you need to do that, since gtk_file_chooser_set_preview_widget adds the widget to the filechooser and thus it's destroyed when the filechooser is destroyed. (And otherwise you'd need to initialise |GtkWidget *img_preview;| to NULL.)
Comment 8 Michael Ventnor 2007-12-17 11:48:04 PST
Created attachment 293547 [details] [diff] [review]
Patch 2
Comment 9 Michael Ventnor 2007-12-18 12:29:45 PST
Comment on attachment 293547 [details] [diff] [review]
Patch 2

This has a problem with images smaller than 160px. New patch soon.
Comment 10 Michael Ventnor 2007-12-18 13:27:17 PST
Created attachment 293732 [details] [diff] [review]
Patch 3

This patch will let GTK scale down images that are too small but keep images that are already smaller than the maximum preview size so that GTK doesn't scale them up. It will also calculate padding to make sure the preview pane is always the same size, with the image centered.
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-12-18 18:28:14 PST
This looks good. I have one question related to Christian's comment #7 though. Since we call gtk_image_new, aren't we responsible for calling gtk_image_destroy? Won't gtk_file_chooser_set_preview_widget add its own reference to the image instead of stealing ours? I sure hope so...
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-12-18 19:21:41 PST
Comment on attachment 293732 [details] [diff] [review]
Patch 3

I guess not, let's go with this.
Comment 13 Reed Loden [:reed] (use needinfo?) 2007-12-18 19:31:35 PST
Comment on attachment 293732 [details] [diff] [review]
Patch 3

Show preview of file in GTK file picker.
Comment 14 Reed Loden [:reed] (use needinfo?) 2007-12-19 03:30:29 PST
Checking in widget/src/gtk2/nsFilePicker.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsFilePicker.cpp,v  <--  nsFilePicker.cpp
new revision: 1.16; previous revision: 1.15

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