Closed Bug 123197 Opened 23 years ago Closed 20 years ago

gtk2 needs to have the filepicker hooked up

Categories

(Core :: XUL, defect)

Product:
Core
Component:
XUL
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8beta1

People

(Reporter: blizzard, Assigned: caillon)

References

Details

Attachments

(6 files, 4 obsolete files)

Patch, dynamically loading symbols
(deleted), patch
caillon
: review+
caillon
: superreview+
Details | Diff | Splinter Review
The right patch
(deleted), patch
blizzard
: review+
blizzard
: superreview+
Details | Diff | Splinter Review
Thsi is based on dynamic loading symbold for Aviary
(deleted), patch
Details | Diff | Splinter Review
from attachement #155072 (for aviary)
(deleted), patch
Details | Diff | Splinter Review
Combined both the patches in one.
(deleted), patch
Details | Diff | Splinter Review
this fixes the issue of gtk file picker not coming up
(deleted), patch
Details | Diff | Splinter Review 
Need to hook up the filepicker with gtk2.
Blocks: gtk2
i was looking into this, and, well, frankly, the GtkFileSelector kinda sucks. 
it doesn't even have a dropdown box to set a file pattern to filter the list. 
one can be added, of course, since GtkFileSelector is just a GtkDialog. 
blizzard: is this what you had in mind for this?  if so, i'll work on this,
unless you or someone else has started on it.
I'm didn't have any specific plans, except it would be nice to use the native
filepicker where possible.  Go right ahead, if you're motivated.
sounds good to me.  i'll let you know how it goes ^_~
sorry i haven't gotten to this sooner...  anyway, i've been playing with the
gtk2 filepicker, and figured out how to add i filter box to the window, but
there are a couple problems.  first of all, there's no way to easily filter the
file listbox.  the gtk api call that i thought would work,
gtk_file_selection_complete() actually applies a filter to both the file _and_
directory list boxes.  i honestly can't think of one useful reason to do this.
also, there's the problem of handling a filter with multiple definitions, such
as "*.jpg; *.gif."  *_complete() doesn't handle this as-is.  i suppose i could
write a routine to filter the file list box based on the filter, using the
semicolon as a separator for multiple extensions in a filter.  in theory tho,
the gtk team will at some point redesign the filepicker, and who knows what it
will look like then. (i'm actually not sure if it is the gtk filepicker or gnome
filepicker that is going to be redesigned.)

so what do we think?  should i go ahead and write up a filtering routine, leave
out the filter box and use the GtkFileSelection as-is, or scrap using a native
dialog and keep moz's internal one for now?  i could also look for a third-party
filepicker that uses gtk2.  thoughts?
Attached patch gtk2 filepicker hookup patch (obsolete) (deleted) — — Splinter Review 
ok, went ahead and implemented my own filtering code.  it appears to work ok,
tho i wasn't really sure how to test it aside from clicking on filters and
navigating to a bunch of directories.

i think i caught all possible mem leaks and checked for all appropriate error
conditions, but it should be looked over by someone other than me, certainly.

apply with -p0 in widget/src/gtk2
Whiteboard: [patch]
Brian, I tested your patch on recently trunk, it can compile, but when mozila
start up, the "Registering nsWidgetGtk2Module components" seems has problem. Can
you test your patch on recent trunk, thanks a lot!

In addition, I have some question about File Picker:

1. What the File Picker do? It seem to be concern the "File->OpenFile...". Under
mozilla/widget/src, gtk module hasn't implemented nsIFilePicker, but
"File->OpenFile..." does work. gtk2 module are same.

2. other modules such as mac, windows implement nsIFilePicker, which is really
used by other part of mozilla.Then why gtk(gtk2) can work properly without them ?

I am so confused by these question. I will be appreciated if someone can give me
some lights.
sorry i've taken a while to get back to this...

the filepicker is indeed the file->save as... or file->open file... box that
pops up.  the one currently in use by gtk and gtk2 builds is part of mozilla's
set of XP widgets.  it's just a bit nicer and more consistent to use native
widgets and dialogs where possible, hence this bug.

as for your runtime issues...  my current CVS isn't compiling, despite
yellow/green status on tinderbox, so i guess i'll try re-pulling the tree or
something, which i don't have time to do right now - perhaps tomorrow.
ok, i figured out what the problem was.  in my first patch i forgot the
modification to widget/src/xpwidgets/Makefile.in, to include
nsBaseFilepicker.cpp for gtk2 builds.  updated the patch in places for the
current trunk (a few trivial differences).  it builds and works fine for me. 
give it a try and let me know what you think.

* NOTE: this patch needs to be applied with 'patch -p0' in widget/src this
time, NOT in widget/src/gtk2 *

(p.s. is there some kind of standard for patches, e.g. should all be made from
the root of the mozilla tree, etc.?)
Attachment #92875 - Attachment is obsolete: true
Just a note: AFAIK there will be a new file chooser in GTK 2.2, currently under
development. New API and totally revamped UI.
Couple of points:
1) GTK2 file dialog will remain API and ABI-compatible with GTk2.2.
2) Filtering is already avaibale - try writing "*.txt" in the input field and
pressing <TAB>.

while this is obviously not the place to debate this (perhaps i'll open a bug on
gtk's bugzilla at some point), i feel the need to offer at least a vague defence
for the work i've done.

> 2) Filtering is already avaibale - try writing "*.txt" in the input field and
> pressing <TAB>.

i find this filtering method totally unnaceptable and unintuitive.  while it is
reminiscent of shell-type tab-completion, that paradigm does not really fit with
a GUI so well:

1) it is not immediately apparent how to "get back" the entries in the file box
that disappeared after a tab-press.  deleting just one character should
re-filter the box, but it does not- another tab-press is needed.
2) this new "tab-completion" combines shell-type tab-completion with
shell-glob-completion as well (e.g., typing "*.txt" at a bash prompt and hitting
<tab> does not give you a list of all files ending in .txt).
3) this also filters the directory list box.  it's quite confusing and unnerving
(at least to me) to see the directories in a file dialog disappear -
tab-completion in a shell may only show directories that match, but a filter box
never touches the directory list.

i think the basic problem of why this doesn't translate is because in a shell
setting, tab completion shows you possibilities when you have none, where this
new GUI-based tab completion attempts to narrow your possibilities when you have
many.  yes it's useful, but i don't think it's intuitive.

the gtk2 method also rules out the possibility of doing what is possible on
other platforms - allowing mozilla to programmatically restrict the file
display.  if the possible filters are "*.swf" and "*.html" then there is no way
to display, for example, a .pdf file.  some people may not like this, but this
is the reality of how it works across platforms.  often the filter box has a
"*.*" option to combat this.  depending on platform you can enter your own
filter into the file selection box, and hit enter.  but the gtk2 box is totally
inconsistent.

my work brings the filepicker in the gtk2 port of mozilla closer to being
consistent with mozilla's filepicker on other platforms.  the debate rages on as
to whether mozilla should be consistent across platforms or try to look and
behave like other apps one might find on the particular platforms.  the work
i've done supports the former (which i don't always agree with, anyway).

on the other hand, i failed to take into account the tab-completion on the gtk2
file box, so my guess is that if you start doing tab-complete while using my
filter box or vice-versa, odd things may happen.  if there is still any interest
in this bug for the time being, i'll look into it.
Can I clarify my earlier comment (in light of recent developments): A new file
selector _is_ being written, but didn't make Gtk/GNOME 2.2 by a long way. It
will be implemented in a future release.
I vote for using Mozilla's builtin filepicker until GTK filepicker stops sucking. 

It seriously lacks multiple column list view with sorting, and column
hiding/unhiding.

It basically sucks visually, too (IMHO). If anything, this bug should not delay
work on Mozilla's GTK2 port in any part.
Blocks: 233462
blizzard is this one that you're working on for the desktop push?
Yeah, it will be.  The API for the filepicker has just settled down now, so I
should be working on this soonish.
*** Bug 223369 has been marked as a duplicate of this bug. ***
this would be a good one to get for 1.8a...  what are the chances?
Flags: blocking1.8a+
Pretty good, but pango comes first.

As a note, you can probably just grab the code out of epiphany.  Marco said that
it was fine with him if we just took it.
Flags: blocking1.8a1+ → blocking1.8a1-
*** Bug 248143 has been marked as a duplicate of this bug. ***
Assignee: blizzard → caillon
Preliminary patch at:
http://people.redhat.com/caillon/patches/mozilla/gtk2-file-chooser/gtk2-file-chooser.patch

Todo:
- Dynamically load symbols

Anyone care to test this a bit?
Status: NEW → ASSIGNED
Whiteboard: [patch]
Target Milestone: --- → mozilla1.8beta
Note: the patch in its current state requires gtk+ 2.4 to build with.  I am
working on making that not the case, but just thought I'd note that :-)
Comment on attachment 153796 [details] [diff] [review]
Patch, dynamically loading symbols

blizzard gave r= over IRC.
Attachment #153796 - Flags: review+
Comment on attachment 153796 [details] [diff] [review]
Patch, dynamically loading symbols

(blizzard actually gave r+sr) I also showed this to a few gtk people and aside
from the "ewww dlsym, you're friggin crazy" comments had no complaints with it.
Attachment #153796 - Flags: superreview+
Looking at the code, it appears to unconditionally use the gtk filepicker
if gtk-2.4 is available.  Shouldn't we allow the theme to pick whether they
want a native or XUL filepicker?  For some themes native is a natural fit,
but for others the native look will clash horribly.

Tor, if you want to do that, let's do that on all platforms.
Is the most-recently-attached patch what landed on the trunk?
Attached patch Supplementary patch (obsolete) (deleted) — — Splinter Review 
- Fixes issue for clobber builds where the file chooser would not always get
registered.  Unfortunately, this still can not be guaranteed to work because of
how XPCOM (mis)handles multiple components registered with the same contract
ID.  See bug 253136.
- The native widget component factory constructor now handles the logic for
which implementation to choose.  This allows me to remove the proxy logic from
the implementation itself.
- Remembers the last selected directory between invocations.
- Removes some debug stuff I forgot to remove.
- Gets rid of the static nsString cruft that got accidentally added while I was
copying over stuff from other implementations.
- Fix a shutdown "leak".
Attached patch The right patch (deleted) — — Splinter Review 
This is the one.
Attachment #155071 - Attachment is obsolete: true
Attachment #155072 - Flags: review+
Attachment #155072 - Flags: superreview+
Going to close this one out.  The main issue for Mozilla AIUI is really just bug
253136.  I'll entertain other issues in new bug reports.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attached patch Patch for Aviary branch (obsolete) (deleted) — — Splinter Review 

    
    

    
  
please ignore my above attachment. 

    
  

        Attachment #155789 -
        Attachment is obsolete: true

    
    

    
  


  

    
    

    
  
Comment on attachment 155805 [details] [diff] [review]
Thsi is based on dynamic loading symbold for Aviary

Chris: can you review these patches for aviary-safety?

        Attachment #155805 -
        Flags: review?(caillon)

    
    

    
  
Comment on attachment 155807 [details] [diff] [review]
from attachement #155072 (for aviary)

Chris: can you review these patches for aviary-safety?

        Attachment #155807 -
        Flags: review?(caillon)

    
    

    
  
These look fine.  I'd rather you just roll them up into one patch than separate
them really.  But according to bryner, it breaks static builds because of the
NS_ERROR_FACTORY_REGISTER_AGAIN stuff.  So I think we need a better solution.

    
    

    
  


  

    
    

    
  
Comment on attachment 156020 [details] [diff] [review]
Combined both the patches in one.

Oh, make sure you get the tweak I added in the final checkin for default button
handling.  You should be able to safely copy the version of the trunk
nsFilePicker.cpp over to the branch

    
    

    
  
I get the following errors in the console when I run the nightly of Thunderbird
of Aug 19, which has the gtk2 filepicker, when I the file open dialog appears


(Gecko:4114): GLib-GObject-WARNING **: invalid cast from `GdkWindow' to `GtkWindow'

(Gecko:4114): Gtk-CRITICAL **: file gtkwindow.c: line 1883
(gtk_window_set_transient_for): assertion `parent == NULL || GTK_IS_WINDOW
(parent)' failed


    
    

    
  


  
(update of attachment 156020 [details] [diff] [review])
This is the update of attachment 156020 [details] [diff] [review] plus the nsFilePicker.[cpp,h] which
caillon@gmail.com  mentioned to take from Mozilla Trunk + it solves the issue
of gtk file picker not comming up.

    
    

    
  
If this checkin caused blocker bug 258347: Please consider backing it out.
The new filepicker barely works on gtk2 builds anyway. If a file has been
created on disk after i have used filepicker once, i have to quit mozilla and
restart to make the filepicker "see" the file. All in all, Mozilla on gtk2 has
become unusable for basic file related tasks. This is bad.

According to Bienvenue, neither he nor Seth Spitzer are the right persons for
bug 258347. If the checkin isn't backed out, whoever caused this serious
regression should take on the assignment.

    
    

    
  
Hey Chris Aillon, can you help drive the fix for the remaining regressions this
bug caused? It looks like someone posted a patch on 8/24 to try to fix the
problem with the gtk2 file picker not coming up for mailnews when adding an
attachment. Is this the right fix? Can you drive it into the tree if so? This is
a pretty nasty regression that we shouldn't still be waiting to get fixed 2
weeks after the fact. Thanks!

    
    

    
  
Can someone assign the right bug to me (read: not this one), and post a trunk
diff of what is required to make this work for people?  I get a sexy file
chooser on my trunk build, so unsure what is going on.

    
    

    
  
Blocker Bug #255900 has been re-assigned to you Chris :) 

But the possible patch to fix the regression is listed here in this bug. 

    
    

    
  
I wouldn't call the filepicker sexy.. it's next to useless here. For one, there
is absolutely no indication  of what are files and what are directories. It's
just a list of names. If i didn't know the directory structure beforehand, I
wouldn't have been able to use it.

    
    

    
  
i found out that the filepicker has icons to indicate folders and files if i use
mozilla under gnome. But not under KDE. Meaning for instance that the filepicker
mozilla uses now is themed by Gnome, not Gtk. It also means that filetypes
aren's seen properly, so the user can't filter on filetypes. This makes Mozilla
needlessly difficult to use under KDE.

Also: It is much more difficult to use under Gnome as well,  because the editor
field in the filepicker has been removed and can not be revoked in any way.
This means I can't type or paste in a file-URL into the filepicker. I will have
to maneuver with innumerable clicks and drags to get to a file.

I strongly recommend this checkin be backed out untill the Gtk filepicker is
pure Gtk, and as usable as the one Mozilla had untill this checkin. (I.E. Files
again can be written to it)

    
    

    
  
Press Ctrl-L to reveal the file-path entry widget, and please don't add any more
comments to this bug regarding filepicker widget advocacy.

    
    

    
  
This patch caused smoketest blocker bug 264210 when the GTK2 filepicker is used...
Depends on: 264210

        Attachment #155805 -
        Flags: review?(caillon)

        Attachment #155807 -
        Flags: review?(caillon)

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: