780530 - Notification after webapp uninstallation

Status: RESOLVED FIXED

(Firefox Graveyard :: Webapp Runtime, defect)

Comment 1

[Marco Castelluccio [:marco]]

Attachment: Patch

http://upstream-tracker.org/versions/libnotify.html

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 652635 [details] [diff] [review]
Patch

Review of attachment 652635 [details] [diff] [review]:
-----------------------------------------------------------------

::: webapprt/gtk2/webapprt.cpp
@@ +272,5 @@
> +    typedef void  (*notify_init_t)(char*);
> +    typedef void* (*notify_notification_new_t)(char*, char*, char*, char*);
> +    typedef void  (*notify_notification_show_t)(void*, char*);
> +
> +    void *handle= dlopen("libnotify.so", RTLD_LAZY);

You need to open libnotify.so.4, and libnotify.so.1 if that fails.

@@ +288,5 @@
> +    nn_init(appName);
> +
> +    void* n = nn_new(removeMsg, NULL, "dialog-information", NULL);
> +
> +    nn_show(n, NULL);

dlclose(handle);

Comment 3

[Marco Castelluccio [:marco]]

Attachment: Patch v2

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 652768 [details] [diff] [review]
Patch v2

Review of attachment 652768 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a comment somewhere about the fact that the only difference between libnotify.so.4 and libnotify.so.1 for the symbols we use is in the definition of notify_notification_new, which takes 3 char* in one and 4 in the other (I don't remember which is which, but it would be good to tell), and that always passing 4 with the 4th being NULL is binary compatible with the one that takes 3.

Please also get review from someone else for the string addition. No idea who would be suitable, though.

::: webapprt/gtk2/webapprt.cpp
@@ +267,5 @@
> +    strcpy(appName, profile);
> +  }
> +
> +  char uninstallMsg[MAXPATHLEN];
> +  if (!NS_FAILED(parser.GetString("Webapp", "UninstallMsg", uninstallMsg, MAXPATHLEN))) {

NS_SUCCEEDED()

@@ +272,5 @@
> +    typedef void  (*notify_init_t)(char*);
> +    typedef void* (*notify_notification_new_t)(char*, char*, char*, char*);
> +    typedef void  (*notify_notification_show_t)(void*, char*);
> +
> +    void *handle= dlopen("libnotify.so.4", RTLD_LAZY);

whitespace before the =.

Comment 5

[Marco Castelluccio [:marco]]

Attachment: Patch v2 with comment

I've already got a review from Myk in another bug for the string addition.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 652808 [details] [diff] [review]
Patch v2 with comment

Review of attachment 652808 [details] [diff] [review]:
-----------------------------------------------------------------

You didn't really need another review from me :)

::: webapprt/gtk2/webapprt.cpp
@@ +267,5 @@
> +  char uninstallMsg[MAXPATHLEN];
> +  if (NS_SUCCEEDED(parser.GetString("Webapp", "UninstallMsg", uninstallMsg, MAXPATHLEN))) {
> +    /**
> +     * The only difference between libnotify.so.4 and libnotify.so.1 for these symbols
> +     * is that notify_notification_new takes three arguments in libnotify.so.4 and 

trailing whitespace here.

Comment 7

[Marco Castelluccio [:marco]]

Attachment: For landing

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=e01235322203

https://hg.mozilla.org/integration/mozilla-inbound/rev/50dd132b8440

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/50dd132b8440