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 516933 - Rewrite attachment UI
Rewrite attachment UI
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
2.22.x (obsolete)
Other Linux
: Normal enhancement
: ---
Assigned To: Matthew Barnes
Evolution QA team
evolution[composer] evolution[attachm...
Depends on: 577898
Blocks: 567284 579099
 
 
Reported: 2008-02-17 00:59 UTC by Matthew Barnes
Modified: 2009-04-27 19:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Illustration of attachment weirdness (70.20 KB, image/png)
2008-12-20 09:24 UTC, Baptiste Mille-Mathias
  Details
Attachment Bar (Icon View) (72.36 KB, image/png)
2009-03-28 03:21 UTC, Matthew Barnes
  Details
Attachment Bar (List View) (71.16 KB, image/png)
2009-03-28 03:22 UTC, Matthew Barnes
  Details
New Composer Window (47.10 KB, image/png)
2009-03-28 03:53 UTC, Matthew Barnes
  Details
First Candidate Patch (609.87 KB, patch)
2009-04-15 13:42 UTC, Matthew Barnes
none Details | Review
Revised patch for older GTK+ versions (610.49 KB, patch)
2009-04-22 21:52 UTC, Matthew Barnes
none Details | Review
crashing eml (7.17 KB, text/plain)
2009-04-23 08:59 UTC, Milan Crha
  Details
Revised patch with bug fixes (592.64 KB, patch)
2009-04-24 15:18 UTC, Matthew Barnes
committed Details | Review
a) and f) (28.19 KB, image/png)
2009-04-27 13:35 UTC, Milan Crha
  Details
test mail (2.18 KB, text/plain)
2009-04-27 13:36 UTC, Milan Crha
  Details

Description Matthew Barnes 2008-02-17 00:59:17 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...
Comment 1 Matthew Barnes 2008-03-11 00:37:19 UTC
Bumping version to a stable release.
Comment 2 Matthew Barnes 2008-04-09 11:13:41 UTC
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.
Comment 3 Milan Crha 2008-04-10 11:39:48 UTC
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).
Comment 4 Matthew Barnes 2008-04-10 11:53:11 UTC
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.
Comment 5 Suman Manjunath 2008-04-11 19:04:07 UTC
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 :-)
Comment 6 Baptiste Mille-Mathias 2008-12-20 09:24:50 UTC
Created attachment 125041 [details]
Illustration of attachment weirdness

Hello,

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.
Comment 7 Matthew Barnes 2009-03-28 02:59:10 UTC
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:

   GInterface
     |
>    +-- EAttachmentView

   GObject
     |
>    +-- EAttachment
     |
     +-- GtkListStore
     |     |
>    |     +-- EAttachmentStore
     |
     +-- GInitiallyUnowned
           |
           +-- GtkObject
                 |
                 +-- GtkWidget
                       |
                       +-- GtkContainer
                             |
                             +-- GtkBin
                             |     |
                             |     +-- GtkWindow
                             |           |
                             |           +-- GtkDialog
                             |                 |
>                            |                 +-- EAttachmentDialog
                             |
                             +-- GtkIconView
                             |     |
>                            |     +-- EAttachmentIconView
                             |
                             +-- GtkPaned
                             |     |
                             |     +-- GtkVPaned
                             |           |
>                            |           +-- EAttachmentPaned
                             |
                             +-- GtkTreeView
                                   |
>                                  +-- EAttachmentTreeView


EAttachment

   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.

EAttachmentStore

   GtkListStore subclass provides API for adding and removing EAttachments,
   as well as running file chooser dialogs for loading and saving.

EAttachmentView

   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
   methods.

EAttachmentTreeView (implements EAttachmentView)

   Displays attachments in a list, similar to Nautilus' "List View".
   Basically just sets up the cell renderers and implements the interface
   methods.

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.

EAttachmentDialog

   The Attachment Properties dialog, split out into its own widget.
Comment 8 Matthew Barnes 2009-03-28 03:21:37 UTC
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
    editor.
Comment 9 Matthew Barnes 2009-03-28 03:22:18 UTC
Created attachment 131535 [details]
Attachment Bar (List View)

Same scenario, in List View.
Comment 10 Matthew Barnes 2009-03-28 03:53:10 UTC
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.
Comment 11 Srinivasa Ragavan 2009-03-30 03:45:09 UTC
Looks nice Matt
Comment 12 Baptiste Mille-Mathias 2009-03-30 07:28:49 UTC
Hey Matthew,

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 :)
Comment 13 Matthew Barnes 2009-03-30 11:51:41 UTC
(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.
Comment 14 Matthew Barnes 2009-04-04 03:32:02 UTC
The patch for this uses GObject property bindings similar to ExoBinding [1].  I've proposed this as an enhancement for Evolution in bug #577898, so marking it as a dependency for this bug.

[1] http://www.xfce.org/documentation/4.2/api/libexo/exo-Binding-Properties-Functions.html
Comment 15 Matthew Barnes 2009-04-11 02:07:30 UTC
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:

  EAttachmentHandlerImage

    Handles image types, including the "Set as Background" menu option.
    Pretty straightforward.  Lives in widgets/misc, alongside all the
    other attachment classes.

  EAttachmentHandlerMail

    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.

  EAttachmentHandlerCalendar

    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
    List" actions.

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.
Comment 16 Matthew Barnes 2009-04-15 13:42:04 UTC
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.
Comment 17 Karl Relton 2009-04-16 10:26:26 UTC
Awesome work.

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?
Comment 18 Matthew Barnes 2009-04-16 19:29:15 UTC
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.
Comment 19 Karl Relton 2009-04-16 20:33:55 UTC
Right - I see now. Thanks for clarification and good work.
Comment 20 Srinivasa Ragavan 2009-04-20 09:43:47 UTC
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. 
Comment 21 Milan Crha 2009-04-20 11:57:50 UTC
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)

and gtk:
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.
Comment 22 Matthew Barnes 2009-04-20 13:22:48 UTC
Those are GTK+ 2.16 features.  I guess I'll have to work around them for the benefit of SUSE customers.
Comment 23 Suman Manjunath 2009-04-21 03:31:30 UTC
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. 
Comment 24 Matthew Barnes 2009-04-21 14:27:55 UTC
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.
Comment 25 Matthew Barnes 2009-04-22 21:52:17 UTC
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.
Comment 26 Milan Crha 2009-04-23 08:59:35 UTC
Created attachment 133164 [details]
crashing eml

crashes evolution every time I open this simple message with one text attachment.

  • #7 segv_redirect
    at main.c line 426
  • #8 <signal handler called>
  • #9 composite_line
    at pixops.c line 640
  • #10 pixops_process
    at pixops.c line 1340
  • #11 _pixops_composite_real
    at pixops.c line 1798
  • #12 _pixops_composite
    at pixops.c line 1865
  • #13 IA__gdk_pixbuf_composite
    at gdk-pixbuf-scale.c line 142
  • #14 apply_emblems
    at gtkicontheme.c line 2829
  • #15 icon_info_ensure_scale_and_pixbuf
    at gtkicontheme.c line 3014
  • #16 IA__gtk_icon_info_load_icon
    at gtkicontheme.c line 3050
  • #17 gtk_cell_renderer_pixbuf_create_themed_pixbuf
    at gtkcellrendererpixbuf.c line 464
  • #18 gtk_cell_renderer_pixbuf_get_size
    at gtkcellrendererpixbuf.c line 548
  • #19 IA__gtk_cell_renderer_get_size
    at gtkcellrenderer.c line 522
  • #20 gtk_cell_view_size_request
    at gtkcellview.c line 339
  • #21 IA__g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #22 g_type_class_meta_marshal
    at gclosure.c line 878
  • #23 IA__g_closure_invoke
    at gclosure.c line 767
  • #24 signal_emit_unlocked_R
    at gsignal.c line 3174
  • #25 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #26 IA__g_signal_emit_by_name
    at gsignal.c line 3071
  • #27 do_size_request
    at gtksizegroup.c line 620
  • #28 _gtk_size_group_compute_requisition
    at gtksizegroup.c line 820
  • #29 IA__gtk_widget_size_request
    at gtkwidget.c line 3695
  • #30 gtk_button_size_request
    at gtkbutton.c line 1135
  • #31 IA__g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #32 g_type_class_meta_marshal
    at gclosure.c line 878
  • #33 IA__g_closure_invoke
    at gclosure.c line 767
  • #34 signal_emit_unlocked_R
    at gsignal.c line 3174
  • #35 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #36 IA__g_signal_emit_by_name
    at gsignal.c line 3071
  • #37 do_size_request
    at gtksizegroup.c line 620
  • #38 _gtk_size_group_compute_requisition
    at gtksizegroup.c line 820
  • #39 IA__gtk_widget_size_request
    at gtkwidget.c line 3695
  • #40 gtk_hbox_size_request
    at gtkhbox.c line 97
  • #41 IA__g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #42 g_type_class_meta_marshal
    at gclosure.c line 878
  • #43 IA__g_closure_invoke
    at gclosure.c line 767
  • #44 signal_emit_unlocked_R
    at gsignal.c line 3174
  • #45 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #46 IA__g_signal_emit_by_name
    at gsignal.c line 3071
  • #47 do_size_request
    at gtksizegroup.c line 620
  • #48 _gtk_size_group_compute_requisition
    at gtksizegroup.c line 820
  • #49 IA__gtk_widget_size_request
    at gtkwidget.c line 3695
  • #50 gtk_html_embedded_size_request
    at gtkhtml-embedded.c line 224
  • #51 IA__g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #52 g_type_class_meta_marshal
    at gclosure.c line 878
  • #53 IA__g_closure_invoke
    at gclosure.c line 767
  • #54 signal_emit_unlocked_R
    at gsignal.c line 3174
  • #55 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #56 IA__g_signal_emit_by_name
    at gsignal.c line 3071
  • #57 do_size_request
    at gtksizegroup.c line 620
  • #58 _gtk_size_group_compute_requisition
    at gtksizegroup.c line 820
  • #59 IA__gtk_widget_size_request
    at gtkwidget.c line 3695
  • #60 html_embedded_real_calc_size
    at htmlembedded.c line 183
  • #61 html_object_calc_size
    at htmlobject.c line 1080
  • #62 html_embedded_object_changed
    at htmlembedded.c line 367
  • #63 IA__g_cclosure_marshal_VOID__VOID
    at gmarshal.c line 77
  • #64 IA__g_closure_invoke
    at gclosure.c line 767
  • #65 signal_emit_unlocked_R
    at gsignal.c line 3244
  • #66 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #67 IA__g_signal_emit
    at gsignal.c line 3034
  • #68 gtk_html_embedded_changed
    at gtkhtml-embedded.c line 98
  • #69 gtk_html_embedded_add
    at gtkhtml-embedded.c line 109
  • #70 IA__g_cclosure_marshal_VOID__OBJECT
    at gmarshal.c line 636
  • #71 g_type_class_meta_marshal
    at gclosure.c line 878
  • #72 IA__g_closure_invoke
    at gclosure.c line 767
  • #73 signal_emit_unlocked_R
    at gsignal.c line 3174
  • #74 IA__g_signal_emit_valist
    at gsignal.c line 2977
  • #75 IA__g_signal_emit
    at gsignal.c line 3034
  • #76 IA__gtk_container_add
    at gtkcontainer.c line 1157
  • #77 efhd_attachment_button
    at em-format-html-display.c line 1817
  • #78 efh_object_requested
    at em-format-html.c line 638
  • #79 html_g_cclosure_marshal_BOOLEAN__OBJECT
    at htmlmarshal.c line 83
  • #80 IA__g_closure_invoke
    at gclosure.c line 767
  • #81 signal_emit_unlocked_R
    at gsignal.c line 3244
  • #82 IA__g_signal_emit_valist
    at gsignal.c line 2987
  • #83 IA__g_signal_emit
    at gsignal.c line 3034
  • #84 html_engine_object_requested_cb
    at gtkhtml.c line 553
  • #85 html_g_cclosure_marshal_BOOLEAN__OBJECT
    at htmlmarshal.c line 83
  • #86 IA__g_closure_invoke
    at gclosure.c line 767
  • #87 signal_emit_unlocked_R
    at gsignal.c line 3244
  • #88 IA__g_signal_emit_valist
    at gsignal.c line 2987
  • #89 IA__g_signal_emit
    at gsignal.c line 3034
  • #90 element_parse_object
    at htmlengine.c line 1635
  • #91 parse_one_token
    at htmlengine.c line 3984
  • #92 new_parse_body
    at htmlengine.c line 1439
  • #93 html_engine_timer_event
    at htmlengine.c line 4934
  • #94 html_engine_flush
    at htmlengine.c line 6909
  • #95 gtk_html_flush
    at gtkhtml.c line 6341
  • #96 emhs_sync_flush
    at em-html-stream.c line 130
  • #97 emss_process_message
    at em-sync-stream.c line 83
  • #98 g_idle_dispatch
    at gmain.c line 4235
  • #99 g_main_dispatch
    at gmain.c line 2144
  • #100 IA__g_main_context_dispatch
    at gmain.c line 2697
  • #101 g_main_context_iterate
    at gmain.c line 2778
  • #102 IA__g_main_loop_run
    at gmain.c line 2986
  • #103 bonobo_main
    from /usr/lib/libbonobo-2.so.0
  • #104 main
    at main.c line 704

Comment 27 Milan Crha 2009-04-23 09:03:19 UTC
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.
Comment 28 Matthew Barnes 2009-04-23 11:34:23 UTC
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.
Comment 29 Matthew Barnes 2009-04-23 11:55:07 UTC
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.
Comment 30 Milan Crha 2009-04-23 12:07:37 UTC
(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
Comment 31 Matthew Barnes 2009-04-23 12:16:01 UTC
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.
Comment 32 Milan Crha 2009-04-23 15:47:41 UTC
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
   changed

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.
Comment 33 Matthew Barnes 2009-04-24 15:18:46 UTC
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
>    changed

This too should be fixed now.
Comment 34 Matthew Barnes 2009-04-24 15:24:38 UTC
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.
Comment 35 Milan Crha 2009-04-27 13:35:24 UTC
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)).
Comment 36 Milan Crha 2009-04-27 13:36:36 UTC
Created attachment 133416 [details]
test mail

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?
Comment 37 Matthew Barnes 2009-04-27 14:35:01 UTC
(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. 

Comment 38 Matthew Barnes 2009-04-27 16:43:51 UTC
(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.
Comment 39 Milan Crha 2009-04-27 17:22:58 UTC
(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.
Comment 40 Matthew Barnes 2009-04-27 19:17:26 UTC
Okay, let's commit this bad boy and see what breaks.
I expect I'll have some follow-up bugs to address...

http://git.gnome.org/cgit/evolution/commit/?id=e377ea5e61171e57f9e892652d0fd1f77953eda8