mrkara / gtranslator

A fork of the original 2-91 branch of Gtranslator from http://projects.gnome.org/gtranslator
GNU General Public License v3.0
1 stars 0 forks source link

[bgo#680399] gtranslator 2.91.5 segfaults on startup (gdl 3.5.x) #74

Open mrkara opened 7 years ago

mrkara commented 7 years ago

Created attachment 219413 Backrace of the segfault

When in preferences is set to use plugins (in my case translation-memory and codeview) and you try to open a po file, gtranslator segfaults immediately using gnome 3.5.x libs. I'm attaching a trace.

Originally reported by Gert Kulyk at https://bugzilla.gnome.org/show_bug.cgi?id=680399

mrkara commented 7 years ago

This seems like a crash in gdl. Try removing the layout.xml in the ~/.config/gtranslator folder.

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

This was the first thing I did before reporting the bug - no change, it still segfaults when I enable any of the afore mentioned plugins.

Originally posted by Gert Kulyk

mrkara commented 7 years ago

FYI, still the same using released gdl 3.5.4 tarball.

Originally posted by Gert Kulyk

mrkara commented 7 years ago

I have tried here. I get a crash too but in gtk_ui_manager_get_widget, so it's not sure that it's the same bug.

Could you try with gdl 3.5.3 and gdl 3.4.2?

I have done some changes in this cycle, there are still some bugs inside.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

With gdl 3.4.2 it works fine. I'll try gdl 3.5.3 later.

Originally posted by Gert Kulyk

mrkara commented 7 years ago

Gdl 3.5.3 works, too. Here you'll need to delete the ~/.config/gtranslator/layout.xml file to get it working, but it works.

Originally posted by Gert Kulyk

mrkara commented 7 years ago

(In reply to comment #6)

Gdl 3.5.3 works, too. Here you'll need to delete the ~/.config/gtranslator/layout.xml file to get it working, but it works.

Thanks. It's possible that you get such bug using a development version of gdl. With GDL 3.5.2, I'm surprise that it's not working. Then, we have made some error when numbering GDL version number so perhaps you were still using the development version instead of GDL 3.5.2. Anyway most of the changes have still not arrived, so it's not that useful to check what's happen in these versions.

I have tried here but it doesn't work whatever is the version of GDL but it doesn't look like a GDL bug.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Gtranslator (current git master) keeps segfaulting using current gdl builds (currently gdl 3.5.91). Right before the segfault I get the following warning:

(gtranslator:4278): GLib-GObject-WARNING **: invalid cast from GdlDockItem' toGtrWindow'

Maybe this helps tracking down the bug?

Originally posted by Gert Kulyk

mrkara commented 7 years ago

I have this here too with the translation-memory plugin, in plugins/translation-memory/gtr-translation-memory-ui.c:

static void showed_message_cb (GtrTab tab, GtrMsg msg, GtrTranslationMemoryUi *tm_ui) { ...

window = gtk_widget_get_toplevel (GTK_WIDGET (tm_ui)); manager = gtr_window_get_ui_manager (GTR_WINDOW (window));

I think the issue is that the toplevel widget is not a GTR_WINDOW so the manager get a wrong value and it crashes.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

If I understand correctly, the dock item should be attached to the window, and the tm ui is inside the dock item. So doing get_toplevel should give me the gtranslator window as it happened before. Has something changed in the docking? Maybe now it is attached as a standalone window instead?

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #10)

If I understand correctly, the dock item should be attached to the window, and the tm ui is inside the dock item. So doing get_toplevel should give me the gtranslator window as it happened before. Has something changed in the docking? Maybe now it is attached as a standalone window instead?

I'm just looking at it. It's working fine if I remove the code loading the previous layout. I think the cause it rather something like the dock object is not attached correctly after loading it.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

I think one issue is that the format for saving a layout is a bit different and a default layout is included in gtranslator.

But I'm not sure it's the main issue as I still have a crash if I keep saving the layout. It seems that it saves a invalid layout, or a layout which is strange enough to make unexpected things.

<?xml version="1.0"?>

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

If the layout format changed, I am all for changing the default one.

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

Although there is something wrong. See that in gtranslator we do not allow to detach the docks, they are always attached, even when they are going to be moved. In this kind of case no matter what gdl should always get them attached, even if the default format is wrong.

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #14)

Although there is something wrong. See that in gtranslator we do not allow to detach the docks, they are always attached, even when they are going to be moved. In this kind of case no matter what gdl should always get them attached, even if the default format is wrong.

The meaning of detach is a bit different in GDL 3.6.0. In fact the widgets are never detach, they are kept in the widget hierarchy but hidden. That's necessary to keep their position. I don't know if can be an issue for gtranslator.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Created attachment 228306 Fix proposal

The main issue seems to be that the layout-changed signal is emitted when a new layout is loaded. I imagine it was no the case in older version of GDL. In gtranslator it is an issue because this layout-changed signal triggers a save of the current layout. At the end it saves an empty layout that cause the crash.

I have fixed a few small issues to make it compile without warning on GDL 3.6.0.

Originally posted by Sébastien Granjoux

Attachment: https://bugzilla.gnome.org/attachment.cgi?id=228306

mrkara commented 7 years ago

Review of attachment 228306:

Looks good. Thanks a lot.

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

Review of attachment 228306:

One minor thing. Why the layout changed signal is triggered when saving the layout? That sounds wrong to me...

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #18)

One minor thing. Why the layout changed signal is triggered when saving the layout? That sounds wrong to me...

I have expected this question a bit :). The layout changed is emitted each time a new window is created or modified. Loading a layout creates new windows and I haven't done anything to block this signal.

I'm agree that it probably makes sense to block it but I'm not sure that there isn't any case where it is annoying.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Well... this case makes it a bit not very discoverable....

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

Hey Sébastien... what has happened with this issue?

Can I help you with something?

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

(In reply to comment #21)

Hey Sébastien... what has happened with this issue? Can I help you with something?

Thanks but I think my work is done.I haven't done lots of check but I think the patch that I have proposed fix the issue correctly. At least it is working much better here.

It is marked as committed but it seems that it is not in the master branch, I don't know why but I think it will be done soon. If you recompile gtranslator, you can try it too and check that it's working.

Then for the discussion about emitting the layout-changed signal while a new layout is loaded, I haven't decided yet what should I do. But it is not urgent, I will try to fix it during this cycle. Anyway, I think it's better to block the signal in the application.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

It is marked to be committed. I hoped that you would commit it :)

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #23)

It is marked to be committed. I hoped that you would commit it :)

Ok, it's done.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Bug 687680 has been marked as a duplicate of this bug.

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

Bug 687614 has been marked as a duplicate of this bug.

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

Bug 687432 has been marked as a duplicate of this bug.

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

Bug 687322 has been marked as a duplicate of this bug.

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

Bug 685845 has been marked as a duplicate of this bug.

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

Created attachment 228963 Wrong layout disposal

Errr... sorry but it doesn't work for me :(

I've cloned and compiled a fresh copy from Git, and the program opens and works properly, but layout is not saved.

I'm testing it under Ubuntu 12.10 (Quantal) up-to-date and when I open GTR, the layout I previously configured (moved the translation memory field, reduced the messages table, etc) is lost, and I have to reorder it again.

Here are all the messages and warnings I get when running GTR with a PO file as an argument:

$ gtranslator accerciser.gnome-3-6.es.po

(gtranslator:31461): Gtk-WARNING **: Theme parsing error: gtk-main.css:45:39: Failed to import: Error al abrir el archivo: No existe el archivo o el directorio

(gtranslator:31461): GLib-GObject-CRITICAL : g_object_unref: assertion `G_IS_OBJECT (object)' failed Message: Dispose tab Message: Dispose translation memory ui Message: Finalize translation memory ui Message: Dispose context Message: Dispose context Message: Dispose view Message: Dispose view Message: Dispose view Message: Dispose view Message: Dispose view Message: Dispose view Message: Dispose view Message: Dispose view Message: Finalize message table Message: Dispose tab Message: Finalize tab Message: window dispose Message: Finalize notebook Message: window dispose ** Message: Finalize window

(gtranslator:31461): GLib-GObject-WARNING **: invalid unclassed pointer in cast to `GtkWidget'

(gtranslator:31461): Gtk-CRITICAL : gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed Message: Disposing app

Originally posted by Daniel Mustieles

Attachment: https://bugzilla.gnome.org/attachment.cgi?id=228963

mrkara commented 7 years ago

(In reply to comment #30)

Errr... sorry but it doesn't work for me :(

No, the patch is working fine. The goal was only to fix the crash. It doesn't save the layout here neither.

I will take a look at this. I'm agree that it's probably linked to my changes in GDL but I haven't planned to work on all projects using it. It's a pity that such critical bug takes so much time to be fixed. Anyway, thanks for your work.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Created attachment 229011 Another fix

Here is another patch fixing the segmentation fault and allowing to save and restore the layout.

It was still the same issue with the layout-changed signal.

GDL 3.6.0 emits the "layout-changed" signal when the layout changes, including when loading a new layout or when new widgets are added at initialization. It seems that older version of GDL were not emitting this signal in such condition.

gtranslator is saving the new layout after each change. At initialization, before loading the previous layout, the current default layout was saved overwriting the one save from the previous run.

This time, I propose a more drastic change. I have don't save the layout after each change anymore. Now the layout is saved only when the windows is destroyed. It means that if gtranslator crashs the layout will not be saved but I hope it's not a big issue.

Originally posted by Sébastien Granjoux

Attachment: https://bugzilla.gnome.org/attachment.cgi?id=229011

mrkara commented 7 years ago

Issue with latest patch: main window part(strings, translation, original) not works as it should.

  1. as far as i remember part which you could drag was in right, now you can drag left side(it's wrong)
  2. in maximized window drag main tab to some position(eg. as far as possible to left border)-> make window windowed-> exit app->launch app->it restores wrong layout, even if you maximize window you won't get same as before exit

Originally posted by

mrkara commented 7 years ago

for the record: "main tab"/"main window part" i mean http://i.imgur.com/XEpWp.jpg

Originally posted by

mrkara commented 7 years ago

Review of attachment 229011:

mmm, I do not get it... how is the layout saved now?

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #33)

Issue with latest patch: main window part(strings, translation, original) not works as it should.

  1. as far as i remember part which you could drag was in right, now you can drag left side(it's wrong)
  2. in maximized window drag main tab to some position(eg. as far as possible to left border)-> make window windowed-> exit app->launch app->it restores wrong layout, even if you maximize window you won't get same as before exit

I have the same problem. When GTR is opened, the messages table doesn't fit all the window's area. I can drag it to the left border and it seems to be ok, but if I close the application or if I jus resize the window, reducing it, the table reduces itself.

The only part of the layout being saved are the Translation Memory and the Source code viewer sidepanels: if I resize or move them, the layout is saved and correctly restored the next time, but not the table message.

Also, I'm still getting some warnings when launching it from the command-line:

(gtranslator:17800): Gtk-WARNING **: Theme parsing error: :2:10: Not using units is deprecated. Assuming 'px'.

(gtranslator:17800): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

(In reply to comment #35)

Review of attachment 229011 [details]: mmm, I do not get it... how is the layout saved now?

Now the layout is saved only when the windows is destroyed.

static void gtr_tab_dispose (GObject * object) { ... if (!priv->dispose_has_run) { save_layout (GTR_TAB (object));

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

(In reply to comment #33)

  1. as far as i remember part which you could drag was in right, now you can drag left side(it's wrong)

Thanks for the picture but I don't understand the issue. If you want to have the "Message Details" and "Translation Memory" windows on the right, you can just drag and drop these windows on the right.

Or the issue is that you want to have these windows on the right in the default layout?

I have delete the file .config/gtranslator/layout.xml. The default layout is a bit strange, there are some empty spaces. I think it's a issue in GDL.

  1. in maximized window drag main tab to some position(eg. as far as possible to left border)-> make window windowed-> exit app->launch app->it restores wrong layout, even if you maximize window you won't get same as before exit

I have some strange behavior of GDL in some condition. I think it's linked to the same issue than above. Empty panes or notebooks should be hidden which is not always the case. I will try to fix in this cycle.

I'm agree that there are issues with GDL and I appreciate to get bug reports. They are the first one on such thing. Anyway this patch is working as expected.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

(In reply to comment #36)

I have the same problem. When GTR is opened, the messages table doesn't fit all the window's area. I can drag it to the left border and it seems to be ok, but if I close the application or if I jus resize the window, reducing it, the table reduces itself.

Yes, this is indeed an issue with GDL.

The only part of the layout being saved are the Translation Memory and the Source code viewer sidepanels: if I resize or move them, the layout is saved and correctly restored the next time, but not the table message.

The size of the empty space is not saved or not restored. If you put a window in this empty space on the left, everything looks fine to me.

Also, I'm still getting some warnings when launching it from the command-line: (gtranslator:17800): Gtk-WARNING **: Theme parsing error: :2:10: Not using units is deprecated. Assuming 'px'.

I get this too, I don't know from where it comes from. I doesn't seem harmful.

(gtranslator:17800): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

I don't get this one, just a backtrace to know from where it comes from would be useful.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

(In reply to comment #38)

(In reply to comment #33)

  1. as far as i remember part which you could drag was in right, now you can drag left side(it's wrong)

Thanks for the picture but I don't understand the issue. If you want to have the "Message Details" and "Translation Memory" windows on the right, you can just drag and drop these windows on the right.

Or the issue is that you want to have these windows on the right in the default layout?

I have delete the file .config/gtranslator/layout.xml. The default layout is a bit strange, there are some empty spaces. I think it's a issue in GDL.

look to another picture, there is two versions side by side. One with your „Another fix“ other without it, both uses gdl 3.6. differences are obviuos. http://i.imgur.com/Ue4gn.jpg

Originally posted by

mrkara commented 7 years ago

(In reply to comment #40)

look to another picture, there is two versions side by side. One with your „Another fix“ other without it, both uses gdl 3.6. differences are obviuos. http://i.imgur.com/Ue4gn.jpg

Ok. With my last patch gtranslator is working as I expect.

Then I think there is a bug in GDL, because the empty space on the left should be hidden. I will try fix it in the next version of GDL. I haven't seen this before on Anjuta.

This empty space is used by two windows named GtrOpenTranPlugin and GtrCharmapPanel. If you don't have them you can edit the layout.xml file with a text editor and remove them with the additional notebook and paned object containing them. It should fix all issues reported here.

I don't know why I don't have these windows (GtrOpenTranPlugin and GtrCharmapPanel). If it's a normal situation, I can remove them in the default layout file.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Removed those lines, that solves all issues.

By default in gtranslator charmap and opentran plugins are disabled, so either in layout.xml theirs space reservation lines should be removed either they must be enabled by default. Imho by default remove those lines in layout.xml it's better workaround.

Originally posted by

mrkara commented 7 years ago

Created attachment 229218 Same than "another fix" with the hidden panes removed in the default layout

My primary goal is to find an acceptable fix in gtranslator which is currently quite unusable due to the changes in GDL. I hope this one is acceptable.

Then, I will try to fix GDL so it can handle correctly the use case of gtranslator but it will take more time.

Thanks for your feedback.

By the way how could I enable charmap and opentrans plugin?

Originally posted by Sébastien Granjoux

Attachment: https://bugzilla.gnome.org/attachment.cgi?id=229218

mrkara commented 7 years ago

(In reply to comment #43)

Created an attachment (id=229218) [details] [review] Same than "another fix" with the hidden panes removed in the default layout

My primary goal is to find an acceptable fix in gtranslator which is currently quite unusable due to the changes in GDL. I hope this one is acceptable.

Then, I will try to fix GDL so it can handle correctly the use case of gtranslator but it will take more time.

Thanks for your feedback.

By the way how could I enable charmap and opentrans plugin?

You can go to the Edit->Preferences menu and select the Plugins tab to enable them.

If I'm not wrong, to enable the Charmap plugin you need to install libgucharmap-dev to compile Gtranslator with support for it (at this momment, I have no access to my laptop, so I can't confirm it).

If not tested the new patch, but it seems to be ok so, if Nacho agrees, you could commit it.

Again, many thanks for your help with this issue (and also thanks to gymka for testing patches and providing feedback) :)

Originally posted by Daniel Mustieles

mrkara commented 7 years ago

(In reply to comment #44)

You can go to the Edit->Preferences menu and select the Plugins tab to enable them.

i think atleast few versions back, gtranslator preferences menu moved to gtranslator->gtranslator(menu above file menu)->preferences.

If I'm not wrong, to enable the Charmap plugin you need to install libgucharmap-dev to compile Gtranslator with support for it (at this momment, I have no access to my laptop, so I can't confirm it) in archlinux these packages named: gucharmap - for charmap plugin json-glib - for open-tran plugin

for the record: current gtranslator version(released, in ftp) not works with gdl 3.6, it simply crashes. people who not reads bugzilla can't use gtranslator already for two weeks(atleast archlinux users, maybe in ubuntu gdl package is still far from 3.6 version). so it would be good if patches would be accepted asap and new version of gtranslator would be released.

tested latest patch, everything works as it should.

Originally posted by

mrkara commented 7 years ago

(In reply to comment #45)

i think atleast few versions back, gtranslator preferences menu moved to gtranslator->gtranslator(menu above file menu)->preferences.

Thanks, I remember it was possible before but I haven't found the menu. I have enabled the Open Tran plugin and it seems to work fine. At least better than what you have reported.

for the record: current gtranslator version(released, in ftp) not works with gdl 3.6, it simply crashes. people who not reads bugzilla can't use gtranslator already for two weeks(atleast archlinux users, maybe in ubuntu gdl package is still far from 3.6 version). so it would be good if patches would be accepted asap and new version of gtranslator would be released.

I'm fully agree and I'm sorry as I have a major responsibility here.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

(In reply to comment #46)

(In reply to comment #45)

i think atleast few versions back, gtranslator preferences menu moved to gtranslator->gtranslator(menu above file menu)->preferences.

Thanks, I remember it was possible before but I haven't found the menu. I have enabled the Open Tran plugin and it seems to work fine. At least better than what you have reported. i reported as is, then you install gtranslator to clean home folder, without any changes. de facto: in my last picture you see what you see then you install gtranslator, there is empty space in left and if you drag notepad part to there after app restart it won't be in same position. ofcourse if you enable additional plugins everything looks ok, but application should work "out of the box", not after tweaking it's settings!

Originally posted by

mrkara commented 7 years ago

Please start pushing the patches. As I said before I still have some concerns about them:

Apart from that I prefer to have them already included so distros can apply them downstream.

Originally posted by Ignacio Casal Quinteiro (nacho)

mrkara commented 7 years ago

(In reply to comment #48)

Please start pushing the patches.

I have pushed it.

As I said before I still have some concerns about them:

  • the layout should be saved always it changes and at the end of the application

I have already replied to that (comment 32 and 37). With the patch that I have just pushed, the layout is saved only at the end of the application. So I think the only difference is that if the application crashes the layout will not be saved. Is it an issue for you or is there something else?

Then the "layout-changed" signal is emitted when the layout is changed, not during the save as you write in comment 18. But it is emitted during the load and I think several times. So the issue is that after the first change, the half loaded layout is saved erasing the current one. I don't know how to fix it in GDL. I will try to see what can be done but it can be quite difficult.

Please report any issue you can find with this patch or more generally with the new version of GDL.

Originally posted by Sébastien Granjoux

mrkara commented 7 years ago

Created attachment 229509 A third fix for this bug

I have spent a bit more time to understand what's happen.

The "layout-changed" signal is emitted each time there is a change in the layout. So if you move a pane division but if you add or remove a new dock item too.

I could perhaps not emit the layout-changed signal if the widget is not realized but I'm not convince it's right. You really dock new widgets, it can be useful to know about it. What do you think?

Again, I'm not sure it's better to not emit the layout-changed signal. The layout manager and the master are still alive. What do you think?

Based on this, here is a patch that is connecting the on_layout_changed signal after loading the layout and disconnect it when destroying the GtrTab widget. I think it's the right fix but tell me if you have another idea.

Originally posted by Sébastien Granjoux

Attachment: https://bugzilla.gnome.org/attachment.cgi?id=229509