GNOME Bugzilla – Bug 516933
Rewrite attachment UI
Last modified: 2009-04-27 19:17:26 UTC
I have no idea how much work would be involved, but I thought it might be cool if Evolution's attachment bar worked more like Nautilus in that you could choose between "icon view" and "list view".
I'm thinking leave icon view alone, it works great as it is. The list view could display at least the filename, size, and MIME type in a table, somewhat similar to Claws Mail.
Perhaps something to consider for the composer rewrite...
Bumping version to a stable release.
Actually, I'm going to broaden the scope of this bug a bit to track a full EAttachmentBar rewrite. There's a few things I want to attempt:
- Port it from GnomeIconList to GtkIconView and call it EAttachmentIconView.
- Port it from GnomeVFS to GIO/GVFS (if Milan doesn't beat me to it).
- Port it from BonoboUI to GtkUIManager.
- Split out a separate EAttachmentStore class.
- Maybe introduce an EAttachmentListView so you can switch between an icon
view and list view, like in Nautilus.
- Try to come up with a container widget that's common to both the composer
and the "comp-editor" used in calendar/tasks/memos. It would handle the
expander, attachment count, maybe an icon/list view combo, etc.
Thanks for the info. I'm fine you'll do the gio/gvfs port there, but in that case you block the bug #526739. How quickly are you able to do that?
I will use your implementation in /calendar/gui/e-meeting-store.c too, it does similar thing (and only there, we have only two async downloads in evo).
I was hoping to start on it tomorrow, but feel free to start on the GVFS part since I'll probably do the widget stuff first.
The only concrete thing I had in mind for the GVFS part is to change the API to always take GFiles rather than filename or URI strings, so you don't need separate functions for remote versus local files.
From bug #517134, the attachment-bar context-menu should also list the "Recent Documents" for quick-add.
This should be very simple if EAttachmentBar is re-written to use GtkUIMananger :-)
Created attachment 125041 [details]
Illustration of attachment weirdness
I would like also to raise the issue where the mailer duplicates attachements informations.
Once above the mail body, once below, this is quite weird.
This effort is fully underway on my kill-bonobo branch, so I guess this is the best place to post screenshots and ask for feedback.
Here's the classes and interfaces I've come up with so far:
> +-- EAttachmentView
> +-- EAttachment
> | +-- EAttachmentStore
| +-- GtkWindow
| +-- GtkDialog
> | +-- EAttachmentDialog
> | +-- EAttachmentIconView
| +-- GtkVPaned
> | +-- EAttachmentPaned
> +-- EAttachmentTreeView
Primarily encapsulates a GFile, GFileInfo and CamelMimePart, as well as
various other attributes. Updates its own row in the tree model, and
provides a GIO-style asynchronous API for loading, saving, and opening
attachments. Builds its own GIcon with any required emblems.
GtkListStore subclass provides API for adding and removing EAttachments,
as well as running file chooser dialogs for loading and saving.
Interface for common attachment view operations, such as selection
management, drag-and-drop, popup menu, etc.
EAttachmentIconView (implements EAttachmentView)
Displays attachments as a grid of icons, similar to what we have now.
Basically just sets up the cell renderers and implements the interface
EAttachmentTreeView (implements EAttachmentView)
Displays attachments in a list, similar to Nautilus' "List View".
Basically just sets up the cell renderers and implements the interface
EAttachmentPaned (implements EAttachmentView)
GtkVPaned subclass with a GtkVBox in the top pane and a notebook of
attachment views in the bottom pane. The attachment bar controls are
packed at the bottom of the top pane, and include an expander, an "Add
Attachment" button, and a view switching combo. The attachment bar
can now be resized.
The Attachment Properties dialog, split out into its own widget.
Created attachment 131534 [details]
Attachment Bar (Icon View)
Simultaneous loading and saving some large audio files on a remote store.
Some things to notice:
- Attachment label shows the total (on-disk) size of attachments.
- Resize grip (between the controls and icon view) allows you to resize
the attachment bar. Position is remembered when expander is toggled.
- Progress bars (and emblems) indicate file transfer progress. These
can be cancelled by right clicking on the icon during transfer.
- The button for adding attachments has moved from the toolbar to the
attachment bar (bug #494629).
- The Icon View / List View combo selection is remembered across sessions,
and is synchronized with the attachment bar in the appointment/task/memo
Created attachment 131535 [details]
Attachment Bar (List View)
Same scenario, in List View.
Created attachment 131538 [details]
New Composer Window
Previous screenshots are a bit busy, and we want to make sure a user will be able to spot the new "Add Attachment" button location.
Here's what a new composer window looks like. I think the button is fairly easy to spot.
Looks nice Matt
this is really awesome, anyway I'd like to do some comments:
- why the download icons are differents in http://bugzilla.gnome.org/attachment.cgi?id=131534 ?
- For the pictures, I would use the mime-type icon rather than the preview as the size is too small to see something. It would would be consistent with other attachment types too, and to preview the pictures or any file, you can click on it .
This is really a good job :)
(In reply to comment #12)
> - why the download icons are differents in
> http://bugzilla.gnome.org/attachment.cgi?id=131534 ?
One attachment is being loaded from remote storage (via Add Attachment), the other is being saved to remote storage (via Save As). The screenshot was meant to demonstrate doing both at the same time.
> - For the pictures, I would use the mime-type icon rather than the preview as
> the size is too small to see something. It would would be consistent with other
> attachment types too, and to preview the pictures or any file, you can click on
> it .
I agree they're a bit too small as is, but I can still adjust the size. Evolution already shows thumbnails for images, PDFs, text files, etc., so I don't want to completely yank that feature. But I -am- doing it differently.
The way it works now is if Nautilus has already thumbnailed the file, I use the existing thumbnail image. Otherwise I just fall back to the standard icon for that MIME type. We don't generate own our thumbnails anymore because that requires either calling deprecated libgnomeui functions or linking to libgnome-desktop, neither of which I want to do.
I've already asked Alex Larsson for a way for applications to request a thumbnail through GIO, which may require breaking the thumbnailing queue out of Nautilus and making it a D-Bus service or something.
The patch for this uses GObject property bindings similar to ExoBinding . I've proposed this as an enhancement for Evolution in bug #577898, so marking it as a dependency for this bug.
More design rambling, as a follow up to comment #7. I'm summarizing here because the patch for this bug currently weighs in at 20K lines and I doubt anyone is really going to read through all that.
First off, I've killed a couple EPlugins. But chill, we're not losing any functionality.
1) "save-attachments" is dead because it's marked experimental and Srini
already implemented it in the core app long ago.
2) Johnny's "import-ics-attachments" plugin has been converted to an
EAttachmentHandler subclass, which is what I wanted to talk about.
EAttachmentHandler is a new way to extend attachment handling. By subclassing EAttachmentHandlerClass, you can extend drag-n-drop, the popup menu, and I think this could eventually replace EMFormatHook (we're not there yet.).
More specifically, EAttachmentHandler is an abstract class that provides an EAttachmentView GObject property and a few virtual methods. Subclasses will typically handle some set of MIME types, much like EMFormatHook plugins do currently. One subclass could handle images. Another subclass could handle mail messages. Another could handle audio formats. etc.
How it works:
Recall that EAttachmentView is a common interface for EAttachmentIconView and EAttachmentTreeView, which helps us avoid rewriting a bunch of duplicate UI code for the attachment "Icon View" and "List View".
When a new icon view or tree view instance is created, EAttachmentView -- in it's initialization routine -- asks GType for a list of registered subclasses of EAttachmentHandler. EAttachmentView then creates one instance of each of these EAttachmentHandler subclasses, and gives each attachment handler a reference back to the attachment view. This lets attachment handlers call EAttachmentView functions.
This turns out to be a really clean way to extend an application. Cleaner than EPlugin, even. If you want to extend Evolution to better handle, say, ZIP file attachments, you just write a new EAttachmentHandler subclass for ZIP files and make sure something somewhere calls your "get_type()" function to register the new GType. And that's it.
I'll write more about how to implement EAttachmentHandler subclasses some other time. For now there's a few built-in subclasses for reference:
Handles image types, including the "Set as Background" menu option.
Pretty straightforward. Lives in widgets/misc, alongside all the
other attachment classes.
This lives in the Mail component because it depends on mail stuff.
It extends the popup menu with "Reply" and "Forward" actions, and
extends drag-and-drop to handle "message/rfc822" and "x-uid-list"
target types. Probably the most complex of the bunch.
Formerly known as the "import-ics-attachments" plugin, this lives
in the Calendar component because it depends on calendar stuff. It
extends the popup menu with "Import to Calendar" and "Import to Task
Once the kill-bonobo branch lands, I can envision other subclasses like EAttachmentHandlerAudio for audio types (to replace the "inline-audio" plugin) and EAttachmentHandlerContacts to handle vCards.
Still testing and debugging all this. Hope to have the patch posted here shortly and maybe land it in 2.27.1.
Created attachment 132694 [details] [review]
First Candidate Patch
Finally have a candidate patch for svn trunk / git HEAD.
Couple known issues:
1) The attachment bar is not sized very well in CompEditor windows.
And somehow my default window size requests are being ignored or
overwritten. No matter. This can largely be mitigated by making
the window size persistent via GConfBridge, which I've already
done on the kill-bonobo branch.
2) The popup menu for inline attachments (via EMFormatHTMLDisplay)
is still using the old EPopup framework. I'm waiting to see what
happens with the Anjal merge before changing too much more here.
For viewing messages with attachments, will there be user option to choose displaying the attachments at the top of the message or the (as it is now) bottom?
The message layout is unchanged. The top attachment view is hidden by default, and is there primarily for users who are more comfortable with drag-and-drop than navigating the file system. You have to click the expander to see it.
Right - I see now. Thanks for clarification and good work.
Its not possible to review completely changes of this volume :-). I would like to just test and take it. Akhil ?
Milan, if you are fine, just glance the code once.
Seems you have quite high requirements on glib version:
e-attachment.c: In function ‘attachment_save_query_info_cb’:
e-attachment.c:2528: error: ‘G_FILE_CREATE_REPLACE_DESTINATION’ undeclared (first use in this function)
e-attachment-paned.c: In function ‘attachment_paned_init’:
e-attachment-paned.c:590: warning: implicit declaration of function ‘gtk_activatable_set_related_action’
e-attachment-paned.c:590: warning: implicit declaration of function ‘GTK_ACTIVATABLE’
e-mail-attachment-bar.c: In function ‘mail_attachment_bar_update_status’:
e-mail-attachment-bar.c:101: error: ‘GtkActivatable’ undeclared (first use in this function)
e-mail-attachment-bar.c:101: error: (Each undeclared identifier is reported only once
e-mail-attachment-bar.c:101: error: for each function it appears in.)
e-mail-attachment-bar.c:101: error: ‘activatable’ undeclared (first use in this function)
e-mail-attachment-bar.c:126: warning: implicit declaration of function ‘GTK_ACTIVATABLE’
e-mail-attachment-bar.c:127: warning: implicit declaration of function ‘gtk_activatable_get_related_action’
e-mail-attachment-bar.c:127: warning: assignment makes pointer from integer without a cast
e-mail-attachment-bar.c:131: warning: assignment makes pointer from integer without a cast
e-mail-attachment-bar.c: In function ‘mail_attachment_bar_init’:
e-mail-attachment-bar.c:591: warning: implicit declaration of function ‘gtk_activatable_set_related_action’
Here I gave up modifying code as I do not have any activatable in my gtk+ 2.14.4.
Those are GTK+ 2.16 features. I guess I'll have to work around them for the benefit of SUSE customers.
Milan - Yes, the GLib requirement is 2.19.10. Akhil and I tried to build kill-bonobo to test this. We hit the same errors. We resorted to upgrading to GNOME 2.26 (on openSUSE 11.1).
We can set up a build repo on build.opensuse.org to build this branch for openSUSE/Fedora. I'll try to do this over the weekend.
I think I can workaround the GTK+ 2.16 symbols for the attachment rewrite.
Not feasible for the kill-bonobo branch. I'll be bumping the version requirements on kill-bonobo as soon as we have a working repo.
Created attachment 133150 [details] [review]
Revised patch for older GTK+ versions
Think I worked around all the GTK+ 2.16 symbols. Again, I'm not making this concession on the branch.
Created attachment 133164 [details]
crashes evolution every time I open this simple message with one text attachment.
Just thinking, does this worth the effort to pushing to trunk "just before" merge of the kill-bonobo branch at all? I know the merge time is still quite unknown, but anyway.
kill-bonobo may yet miss the 2.27.3 deadline, so I do think it's worth it.
The more pieces I can merge early, the easier it is to maintain the branch.
That crash in comment #26 looks like it's coming from deep within GDK.
I'd suggest making sure your GTK+ library is up-to-date.
(In reply to comment #29)
> That crash in comment #26 looks like it's coming from deep within GDK.
> I'd suggest making sure your GTK+ library is up-to-date.
Yup, that's the reason of comment #27. if it's inside Gtk+/Gdk and you will require newest Gtk+ with kill bonobo, but you said you do not want at the moment on trunk, then it seems it comes against each other and newest version is somehow required anyway.
Seems it's fixed half a year back, see bug #557059
A fix was committed to the gtk-2-14 branch according to the bug, so you should just need the latest GTK+ 2.14 update.
Yup, I did and it gone.
Just a quick smoke test:
a) too short file names seem to be aligned to the left
b) attaching a text file without any extension leads to a runtime warning
Gtk-WARNING **: could not load image: Icon not present in theme
and no icon in the attachment bar, only possibility to select the
attachment is to click on its name (other can when on its icon)
c) unfolded attachment bar in the composer seems to share the place with the
message body itself, in some strange ratio, as changing window height
changes height of both the text area nd the attachment panel. As I started
with rather smaller window the attachment bar seemed to be incomplete (I saw
only icons, no captions there).
d) in calendar, appointment creation, unfolding it makes it quite small, and
changing the size is only possible by the splitter below the attachment
controls, which is kinda unusual. Also, the requested height for the icon
view with one attachment and one line of file name and one line of size
information seems to be too tall, approximately two more lines of text are
required to get rid of the right scrollbar of the attachment area.
e) noticed in c) on console:
Gtk-WARNING **: attach-recent: missing action attach-recent
f) not sure how much you can influence, but saving the appointment from c) with
three entered attachments and reopening it overlaps file names there. There
is used an URL to local file, as ECal doesn't support inline attachments.
g) There left an "Attach" icon in the calendar toolbar. Maybe intentionally.
h) changing description and "suggest show inline" isn't stored in a draft,
also description used to be shown immediately, instead of file name, when
I'm not sure how much usable are there Properties in the calendar attachment bar. Doesn't seem to be used in the calendar. Though it's there even now, so no worry about that.
Created attachment 133257 [details] [review]
Revised patch with bug fixes
Thanks for testing this so thoroughly!
> a) too short file names seem to be aligned to the left
I think that's a GTK bug. I'm using GtkIconView in a slightly unconventional way and I think that's revealing some rendering bugs. I plan to file a bug report about it once I have a better understanding of what's causing it.
> b) attaching a text file without any extension leads to a runtime warning
> Gtk-WARNING **: could not load image: Icon not present in theme
> and no icon in the attachment bar, only possibility to select the
> attachment is to click on its name (other can when on its icon)
Should be fixed. It will fall back to a default icon ("mail-attachment") if the MIME type icon can't be found.
> c) unfolded attachment bar in the composer seems to share the place with
> the message body itself, in some strange ratio, as changing window
> height changes height of both the text area nd the attachment panel.
> As I started with rather smaller window the attachment bar seemed to
> be incomplete (I saw only icons, no captions there).
It was a packing issue. Should be fixed in the composer. CompEditor still has some issues (see below).
> d) in calendar, appointment creation, unfolding it makes it quite small,
> and changing the size is only possible by the splitter below the
> attachment controls, which is kinda unusual. Also, the requested height
> for the icon view with one attachment and one line of file name and one
> line of size information seems to be too tall, approximately two more
> lines of text are required to get rid of the right scrollbar of the
> attachment area.
Fixing (c) addressed the initial height issue, but now the window height grows when you expand the attachment view. I think the default window size is just too small, but it seems to be ignoring my size requests. Still investigating.
> e) noticed in c) on console:
> Gtk-WARNING **: attach-recent: missing action attach-recent
Should be fixed now.
> f) not sure how much you can influence, but saving the appointment
> from c) with three entered attachments and reopening it overlaps
> file names there. There is used an URL to local file, as ECal doesn't
> support inline attachments.
Not sure I follow you. Screenshot?
> g) There left an "Attach" icon in the calendar toolbar. Maybe intentionally.
That was an oversight. Should be gone now.
> h) changing description and "suggest show inline" isn't stored in a draft,
> also description used to be shown immediately, instead of file name, when
This too should be fixed now.
Quick note about the patch in comment #33. I pulled the "e-binding.[ch]" files that were in previous patch revisions because I committed those files to master yesterday under a different bug.
Created attachment 133415 [details]
a) and f)
What I see for the a) (the upper part - I'm sorry, I'm a driver, whenever I say left I mean right and vice versa) and for b), which is just opened appointment which has some attachments.
ad d) I think it's fine, though still quite tall (see the image for a)).
Created attachment 133416 [details]
this is for some different bug, but when I unfold attachment bars (there are two), then each of them has different width. Maybe a packing issue in the message pane?
(In reply to comment #35)
> What I see for the a) (the upper part - I'm sorry, I'm a driver, whenever I say
> left I mean right and vice versa) and for b), which is just opened appointment
> which has some attachments.
I'd say those are both GtkIconView bugs.
> ad d) I think it's fine, though still quite tall (see the image for a)).
I'm not setting the attachment bar height there. I'm giving the widget its full height requisition, and that's what GTK+ calculates for it.
(In reply to comment #36)
> this is for some different bug, but when I unfold attachment bars (there are
> two), then each of them has different width. Maybe a packing issue in the
> message pane?
Yeah, I see it too. Reminds me of Srini's observations in bug #312033 comment #9. The Anjal method of packing everything into a normal GtkVBox and using multiple HTML widgets (as opposed to our embedded kludge) is much cleaner and should fix all the resizing issues. (I wonder if we couldn't do that now with GtkHTML -- just stop using GtkHTMLEmbedded.)
I'll see what I can do in the meantime, but I don't consider it a blocker.
(In reply to comment #38)
> I'll see what I can do in the meantime, but I don't consider it a blocker.
Nono, me neither, feel free to commit (I do not feel brave enough to read the whole patch). Meanwhile will srag commit his changes towards WebKit, which I hope do the same as you just described for GtkHTML too. If not, then maybe later.
Okay, let's commit this bad boy and see what breaks.
I expect I'll have some follow-up bugs to address...