Created attachment 187505 [details] [review]
009-name-selector-fixes.patch

This is the patch applying master changes for solving the original bug

I cannot tell where the changes in 009-name-selector-fixes.patch come from. Some of these changes are *not* in e-d-s HEAD, it seems?

Ideally, I would like to have a set of upstream commits that I can cherry-pick with 'git cherry-pick -x' into the 2.32 branch and test.

Would be nice to CC ubuntu maintainers here, but I don't know their mail alias :-S

(In reply to comment #3)
> Would be nice to CC ubuntu maintainers here, but I don't know their mail alias

Maybe seb128 could help here?

So yeah, looking back at this, seems like I had found a crash where the name selector would crash evo if you clicked on the From: or CC: buttons while composing a message. It was very much a temporary thing due to Ubuntu using (IIRC, gtk 2.23 at the time, maybe something later than that) with evo 2.32; so never really expected it to be needed anywhere else.

In any case, this looks to me like equivalent to:
http://git.gnome.org/browse/evolution-data-server/commit/?id=89b384cfe452ce1e38b31066c3d9f9abd1bdd6e5
and
http://git.gnome.org/browse/evolution-data-server/commit/?id=fec059d9e81cb3966c360b2e7115cbe224f50b9a

It's just (looking back) a pretty hackish way to achieve roughly the same thing, except by creating a new GtkComboBoxText rather than the proper way of using that kind of widget in the ui file. That *may* have been because that widget wasn't available for me to use in the UI definitions, I'm no longer sure, and I can't seem to find proof of this in api docs (can't get to the 2.23 versions).

My sincerest apologies for the confusion I've caused. :)

So you'll be happy if I just cherry-pick those two commits?

Anything else you need in a 2.32 update that we haven't picked up so far?

Seems fine by me. FWIW, the other patches we carry in Ubuntu in EDS are the following:

299-git-find_book_by_contact_crash_9b01440.patch
299-git-https_only_webdav_4913827.patch
299-git-maildir_expunge_e7201c8.patch

I see they've all already been cherry-picked to the gnome-2-32 branch.

What does left on this bug, please? Should I mark bug #649854 as a duplicate of this one or vice versa or something else?

I have cherry-picked the two commits mentioned in comment 5, to cope with the GtkComboBoxText requirement in GTK3. We can close this bug, I think.

http://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-2-32&id=3b146b852d2620aa8632afa539adad3c31870764

This one causes evolution to be closed when trying to create a new appointment and using gtk+-2.24.5

The following error is printed when evolution fails:

(evolution:13566): Gtk-CRITICAL **: IA__gtk_combo_box_text_get_active_text: assertion `GTK_IS_COMBO_BOX_TEXT (combo_box)' failed


Maybe this is caused by unproper version in:

+#if !GTK_CHECK_VERSION (2,23,0)

?

Thanks a lot for your help

(In reply to comment #10)
> The following error is printed when evolution fails:
> 
> (evolution:13566): Gtk-CRITICAL **: IA__gtk_combo_box_text_get_active_text:
> assertion `GTK_IS_COMBO_BOX_TEXT (combo_box)' failed
> 
> 
> Maybe this is caused by unproper version in:
> 
> +#if !GTK_CHECK_VERSION (2,23,0)

I do not think so, it's rather that the change category_combo is not of the type it should be. This combo is created in e_name_selector_dialog_init(), and I have there:
> 	combobox_category = gtk_combo_box_text_new ();

Check whether you've it there too, and whether the gtk-compat.h skips redefinition of it. It may not define it, because the newer gtk is defining it natively.

(In reply to comment #11)
> I have there:
> > 	combobox_category = gtk_combo_box_text_new ();
> 
> Check whether you've it there too, and whether the gtk-compat.h skips
> redefinition of it. It may not define it, because the newer gtk is defining it
> natively.

In libedataserverui/e-name-selector-dialog.c (2.32.3 version) I don't have any "combobox_category" string

In gtk-compat.h, I have:
#define gtk_combo_box_text_new			gtk_combo_box_new_text

from patch causing the issues

Oh, it's that old. The 2.32.3 has e-name-selector-dialog.ui, where is defined a widget of an id "combobox-category", which is of type GtkComboBox. For gtk+ 2.24.0+ you want to make it GtkComboBoxText.

It just shows that some changes needs other changes to be complete :)

I am not sure if I understood you properly :-(

Is the problem then in here?:

+#define gtk_combo_box_text_get_active_text	gtk_combo_box_get_active_text

(in libedataserverui/gtk-compat.h)

no, this code is correct, because older gtk+ doesn't have that function. The problem is that the .ui file defines the corresponding widget as GtkComboBox, but with newer gtk+ the code expects GtkComboBoxText. Thus the .ui file should be also conditional, to have this widget once as GtkComboBox (for older gtk+) and as GtkComboBoxText (for newer gtk+). I do not think this is possible for .ui files, thus the easiest way is to change the widget type based on the knowledge against which gtk+ is evolution compiled.

Pacho, AFAIK what is in git now should just work for gtk+ 2.24.5. I haven't tested it, but the code appears to be the way it should for it to work in that version. Now, I can't say for gtk+ versions below 2.24; but I would pretty much expect those to work as well with what's currently defined in gtk-compat.h in git. What appears to me is that at least the two patches initially referred to have been included.

The only thing I'm not completely sure is whether the definition of GtkComboBoxText in gtk-compat.h applies to the UI file once loaded, but I expect it does; hence why I expect the code in git to work as-is. But again, I haven't personally tested it.

I guess the best would be: can you confirm exactly which release of E-D-S you are building, and which patches are included (e.g., if you're cherry-picking anything from git, or including extra patches, etc.).

(In reply to comment #16)
> Pacho, AFAIK what is in git now should just work for gtk+ 2.24.5. I haven't
> tested it, but the code appears to be the way it should for it to work in that
> version.

It fails with gtk+-2.24.5 (it's the one I am using and able to reproduce the problem) :-(

> Now, I can't say for gtk+ versions below 2.24; but I would pretty much
> expect those to work as well with what's currently defined in gtk-compat.h in
> git. What appears to me is that at least the two patches initially referred to
> have been included.
> 
> The only thing I'm not completely sure is whether the definition of
> GtkComboBoxText in gtk-compat.h applies to the UI file once loaded, but I
> expect it does; hence why I expect the code in git to work as-is. But again, I
> haven't personally tested it.
> 
> I guess the best would be: can you confirm exactly which release of E-D-S you
> are building, and which patches are included (e.g., if you're cherry-picking
> anything from git, or including extra patches, etc.).

I am using 2.32.1 with the newer patches committed to the branch:
http://git.gnome.org/browse/evolution-data-server/log/?h=gnome-2-32

And dropping:
http://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-2-32&id=3b146b852d2620aa8632afa539adad3c31870764

(and http://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-2-32&id=26ff15a20e4ffd3f6e02777431a2d86db406f86d as it requires the previous one) solves the problem :-/

(In reply to comment #15)
> no, this code is correct, because older gtk+ doesn't have that function. The
> problem is that the .ui file defines the corresponding widget as GtkComboBox,
> but with newer gtk+ the code expects GtkComboBoxText. Thus the .ui file should
> be also conditional, to have this widget once as GtkComboBox (for older gtk+)
> and as GtkComboBoxText (for newer gtk+). I do not think this is possible for
> .ui files, thus the easiest way is to change the widget type based on the
> knowledge against which gtk+ is evolution compiled.

OK, then, the problem is in:

                <child>
                  <object class="GtkComboBox" id="combobox-category">
                    <property name="visible">True</property>
                    <property name="add_tearoffs">False</property>
                    <property name="focus_on_click">True</property>
                    <property name="model">model1</property>
                    <child>
                      <object class="GtkCellRendererText" id="renderer1"/>
                      <attributes>
                        <attribute name="text">0</attribute>
                      </attributes>
                    </child>
                  </object>
                  <packing>
                    <property name="left_attach">1</property>
                    <property name="right_attach">2</property>
                    <property name="top_attach">1</property>
                    <property name="bottom_attach">2</property>
                    <property name="x_options">fill</property>
                    <property name="y_options">fill</property>
                  </packing>
                </child>

no?

Should "GtkComboBox" be replaced with "GtkComboBoxText" for gtk+-2.24.x then?

That is not the problem as we are also applying:
http://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-2-32&id=26ff15a20e4ffd3f6e02777431a2d86db406f86d

and doesn't solve the problem :(

Any new idea that I could try? If this cannot be solved for 2.32:
- Maybe you could commit original ubuntu patch that looks to not have these issues:
https://bugzilla.gnome.org/attachment.cgi?id=187504
- Per comment #5 maybe no change is needed  but, in that case, that commits should be reverted in 2.32 branch as, currently, that branch is broken and the problem will appear on any new 2.32.x release that could appear in the future

I suppose this is obsolete now, I'm closing it as such.