New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mate-notification-daemon 1.12.0 breaks Pidgin libnotify #91

Closed
Smile4ever opened this Issue Nov 19, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@Smile4ever

This was observed on Manjaro Linux (Arch Linux):

  • 1.12.0 breaks Pidgin libnotify functionality
  • 1.10.2 works perfectly, showing new messages as a notification as they come in
@raveit65

This comment has been minimized.

Show comment
Hide comment
@raveit65

raveit65 Nov 19, 2015

Member

any abnormals in logs?
Did you you file out a report to your distro, to be shure it isn't distro related?

Member

raveit65 commented Nov 19, 2015

any abnormals in logs?
Did you you file out a report to your distro, to be shure it isn't distro related?

@Smile4ever

This comment has been minimized.

Show comment
Hide comment
@Smile4ever

Smile4ever Nov 22, 2015

Which logs?

I tested Linux Mint 17.3 (RC/beta) as well. The same issue is present.

Which logs?

I tested Linux Mint 17.3 (RC/beta) as well. The same issue is present.

@assen-totin

This comment has been minimized.

Show comment
Hide comment
@assen-totin

assen-totin Nov 23, 2015

Notification daemon crashes with SIGSEGV every time an image in added to existing notification. See https://bugzilla.redhat.com/show_bug.cgi?id=1284651 for POC code. This is probably your offending commit: https://github.com/mate-desktop/mate-notification-daemon/pull/80/files
Please, remove it as you break the libnotify C API.

Notification daemon crashes with SIGSEGV every time an image in added to existing notification. See https://bugzilla.redhat.com/show_bug.cgi?id=1284651 for POC code. This is probably your offending commit: https://github.com/mate-desktop/mate-notification-daemon/pull/80/files
Please, remove it as you break the libnotify C API.

@raveit65

This comment has been minimized.

Show comment
Hide comment
@raveit65

raveit65 Nov 24, 2015

Member

confirmed, after reverting the commit a image displays well with the POC.
@GiedriusS @monsta
what do you think?
Can the commit improve or simply revert?

Member

raveit65 commented Nov 24, 2015

confirmed, after reverting the commit a image displays well with the POC.
@GiedriusS @monsta
what do you think?
Can the commit improve or simply revert?

@raveit65 raveit65 added the confirmed label Nov 24, 2015

@raveit65

This comment has been minimized.

Show comment
Hide comment
@raveit65

raveit65 Nov 24, 2015

Member

@Smile4ever
a patch is attached at rhbz report for testing

Member

raveit65 commented Nov 24, 2015

@Smile4ever
a patch is attached at rhbz report for testing

@GiedriusS

This comment has been minimized.

Show comment
Hide comment
@GiedriusS

GiedriusS Nov 24, 2015

Contributor

Yeah, lets revert for now. That indeed breaks the C API :( Sample test code I've used for reproducing this: https://gist.github.com/GiedriusS/c2504a864a472524e4ce

Contributor

GiedriusS commented Nov 24, 2015

Yeah, lets revert for now. That indeed breaks the C API :( Sample test code I've used for reproducing this: https://gist.github.com/GiedriusS/c2504a864a472524e4ce

@monsta

This comment has been minimized.

Show comment
Hide comment
@monsta

monsta Nov 24, 2015

Member

Confirmed, it also breaks test script from #51. Will revert it now.

Member

monsta commented Nov 24, 2015

Confirmed, it also breaks test script from #51. Will revert it now.

monsta added a commit that referenced this issue Nov 24, 2015

@monsta monsta closed this in b297c7d Nov 24, 2015

@monsta

This comment has been minimized.

Show comment
Hide comment
@monsta

monsta Nov 24, 2015

Member

@raveit65: https://bugzilla.redhat.com/1281403 seems to be the same issue (crashes in the same place).

Member

monsta commented Nov 24, 2015

@raveit65: https://bugzilla.redhat.com/1281403 seems to be the same issue (crashes in the same place).

@raveit65

This comment has been minimized.

Show comment
Hide comment
@raveit65

raveit65 Nov 24, 2015

Member

thx, monsta, this report was falling under my radar :)
I'm still wondering why this line wasn't ported to use GArray

struct_type = dbus_g_type_get_struct("GValueArray", G_TYPE_INT, G_TYPE_INT, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT, G_TYPE_INT, dbus_g_type_get_collection ("GArray", G_TYPE_UCHAR), G_TYPE_INVALID);

Member

raveit65 commented Nov 24, 2015

thx, monsta, this report was falling under my radar :)
I'm still wondering why this line wasn't ported to use GArray

struct_type = dbus_g_type_get_struct("GValueArray", G_TYPE_INT, G_TYPE_INT, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT, G_TYPE_INT, dbus_g_type_get_collection ("GArray", G_TYPE_UCHAR), G_TYPE_INVALID);

monsta added a commit that referenced this issue Dec 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment