Update shipped Growl framework to 1.2.2 for compatibility with Growl 1.3

VERIFIED FIXED in Firefox 11

Status

()

Toolkit
XUL Widgets
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Ben, Assigned: mstange)

Tracking

Trunk
mozilla11
All
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox9-, firefox10+, firefox11+ verified)

Details

(Whiteboard: [inbound][qa!], URL)

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a2) Gecko/20111003 Firefox/9.0a2
Build ID: 20111003042017

Steps to reproduce:

I upgraded to Growl 1.3 in the Mac App Store.

The underlying Growl framework has been updated.


Actual results:

I know longer receive Growl updates from Firefox.


Expected results:

I should get notifications.
(Reporter)

Updated

6 years ago
(Assignee)

Comment 1

6 years ago
Looks like we need to update our bundled Growl framework to 1.2.2.
The last update was in bug 522511, as far as I can tell.
Status: UNCONFIRMED → NEW
Component: General → XUL Widgets
Ever confirmed: true
Product: Firefox → Toolkit
QA Contact: general → xul.widgets
Hardware: x86 → All
Version: 9 Branch → Trunk
> Looks like we need to update our bundled Growl framework to 1.2.2.

So we don't need to upgrade it to 1.3 to gain compatibility with Growl 1.3?
Duplicate of this bug: 691892
(Assignee)

Comment 4

6 years ago
Doesn't look like it. There's no framework with version 1.3 yet, and I've confirmed that Growl 1.3 works with Growl.framework 1.2.2.
(Assignee)

Comment 5

6 years ago
Created attachment 564776 [details] [diff] [review]
part 1: update growl framework

I think this is all that's needed. It compiles and works (after applying the next patch which I'm about to attach).

I've followed the steps from bug 522511 comment 3 with minor changes, since the necessary files are now stored in two different directories in the framework archive.

1. I downloaded http://growl.googlecode.com/files/Growl-1.2.2-src.tbz and
   unpacked it.
2. I copied all files from Growl-1.2.2-src/Framework/Source/ and from
   Growl-1.2.2-src/Common/Source/ to toolkit/components/alerts/mac/growl/,
   replacing the existing files. I did *not* hg add any files.
3. I re-added the #import <Cocoa/Cocoa.h> line in front of the #import block in
   GrowlPathUtilities.m.
4. I looked through the diff and searched for any new #import lines, so that I
   could hg add any header files that might have become required in the new
   version of the framework.
   There were none, so I didn't add any.
5. I confirmed that the new tree compiled.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #564776 - Flags: review?(smichaud)
(Assignee)

Comment 6

6 years ago
Created attachment 564778 [details] [diff] [review]
part 2: ignore whether growl is installed or running

With Growl 1.3, [GrowlApplicationBridge isGrowlInstalled] always returns NO. I've asked in the newsgroups about this [1], and the answer I got was that we should ignore whether Growl is installed and just fire notifications unconditionally.

So this patch backs out bug 381520.

With this patch I've confirmed that things work by executing the following from the error console:

Components.classes["@mozilla.org/alerts-service;1"].getService(Components.interfaces.nsIAlertsService).showAlertNotification(null, "title", "text", false, "", null);

[1] http://groups.google.com/group/growl-development/browse_thread/thread/1bf5b4c50375458#msg_bc88df32b77cd771
Attachment #564778 - Flags: review?(sdwilsh)
(Assignee)

Comment 7

6 years ago
Created attachment 564782 [details] [diff] [review]
alternative part 2: only ignore whether growl is installed, still throw exception when growl isn't running

Actually, [GrowlApplicationBridge isGrowlRunning] still works, so we can just keep using it. This way we don't really follow Peter's advice, though.
I'm leaving the choice up to you, sdwilsh.
Attachment #564782 - Flags: review?(sdwilsh)
(Assignee)

Comment 8

6 years ago
Created attachment 564789 [details] [diff] [review]
part 1b: update growl license
Attachment #564789 - Flags: review?(gerv)
(Assignee)

Updated

6 years ago
Summary: Growl 1.3 introduces new framework, breaks Firefox support. → Update shipped Growl framework to 1.2.2 for compatibility with Growl 1.3
Comment on attachment 564776 [details] [diff] [review]
part 1: update growl framework

This looks fine to me.  Thanks for doing it.

One small nit:  You can probably hg remove the following files:

toolkit/components/alerts/mac/growl/GrowlPreferencesController.h
toolkit/components/alerts/mac/growl/GrowlTicketController.h
Attachment #564776 - Flags: review?(smichaud) → review+
One more thing:  Someone should test these changes with older versions of Growl.
Will this be included in one of the next small updates for Thunderbird 7.x or do we have to wait for Thunderbird 8?

Comment 12

6 years ago
I would like to add to comment 7, when will this take place?

Comment 13

6 years ago
(In reply to joel from comment #12)
> I would like to add to comment 11, when will this take place?

I menat comment 11!
(Assignee)

Comment 14

6 years ago
Depends on when it's reviewed. But it will probably have to wait at least for Thunderbird 8, if not longer.

Shawn, when do you think you'll get to the review? Should I ask somebody else for review?
Attachment #564789 - Flags: review?(gerv) → review+
(In reply to Markus Stange from comment #14)
> Shawn, when do you think you'll get to the review? Should I ask somebody
> else for review?
Sorry; I should get to it today.
Comment on attachment 564782 [details] [diff] [review]
alternative part 2: only ignore whether growl is installed, still throw exception when growl isn't running

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

r=sdwilsh
Attachment #564782 - Flags: review?(sdwilsh) → review+

Comment 17

6 years ago
FYI, Growl SDK 1.3 has been released, which no longer relies on Growl.app. So you can display notifications with Mist (basically the SDK, AFAIU), and Growl.app turned into a "pro" frontend for customizing messages. 

Would be nice to see SDK 1.3 implemented! 


http://growl.info/notetodevelopers
Comment on attachment 564778 [details] [diff] [review]
part 2: ignore whether growl is installed or running

Meant to say no on this one.
Attachment #564778 - Flags: review?(sdwilsh) → review-
Markus - ETA on landing this?  Also, this is broken with new growl, so we may want to take this in aurora/beta.
tracking-firefox10: --- → ?
tracking-firefox11: --- → ?
tracking-firefox9: --- → ?
(Assignee)

Comment 20

6 years ago
Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11ed523f1ea2
https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb3558c97ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee9ab83779a9
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/11ed523f1ea2
https://hg.mozilla.org/mozilla-central/rev/4eb3558c97ad
https://hg.mozilla.org/mozilla-central/rev/ee9ab83779a9
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Comment 22

6 years ago
In what version of Thunderbird will this take effect if Firefox is version 11?

Comment 23

6 years ago
AFAICS this is not SDK 1.3. Shall I open another ticket for the new version?

Comment 24

6 years ago
this fix isn't slated until firefox 11!?!?! i'm sorry, but that is pretty ****. i realize you guys have a lots of other things to do, but come on. 

it really makes me start to wonder why this code is even part of the the native release instead of being handled via some kind of extension that could be updated and released more quickly then this fix is going to be.
(In reply to Jae Gangemi from comment #24)
> this fix isn't slated until firefox 11!?!?! i'm sorry, but that is pretty
> ****. i realize you guys have a lots of other things to do, but come on. 

Not sure why "having other things to do" is relevant? This has already been fixed and landed on mozilla-central, which is currently where new development work is occurring (new features do not land on aurora or beta). 

If you wish to try it out, please download the latest nightly: http://nightly.mozilla.org/ or alternatively within 4-5 weeks it will be on aurora: http://www.mozilla.org/firefox/aurora/

If you'd rather not use a nightly/Aurora/Beta, then Firefox 11 final will be released in 17 weeks time.

Comment 26

6 years ago
i don't see how this is a new feature. it's updating an existing library for an already existing feature, but ok, you guys are in charge of your development process, so i'm not belabor that point.

17 weeks is a long time to wait for an existing feature to start working again and i am not interested in working with a nightly as i find the betas to be unstable enough as it is. 

so back to the question of why is this code even in the core of mozilla instead of being handled by some kind of extension that can be updated and released independently of the core?
I think it's too late in FF9 to take on beta, but is this patch too risky to uplift to aurora 10? If not, please nominate for approval on aurora - it'd be good to get notifications working again for up-to-date Growl users on OS X.
tracking-firefox10: ? → +
tracking-firefox11: ? → +
tracking-firefox9: ? → -

Comment 28

6 years ago
When will the update hit Thunderbird, I need that more than Firefox.

Comment 29

6 years ago
again i would like to raise the question of why this code is in the core at all instead of being handled through a separate extension that could be updated and released outside of the mozilla development cycle.

Updated

6 years ago
Blocks: 711822
No longer blocks: 711822
Depends on: 711822

Comment 30

6 years ago
firefox does not properly register with growl upon start up, so i am unable to receive notifications.

https://bugzilla.mozilla.org/show_bug.cgi?id=719485

Comment 31

6 years ago
I thought GROWL would be working in Beta 11 of Thunderbird, but n dice here. Any ideas?

Comment 32

5 years ago
Growl 1.3 has been criticized a lot for two reasons:

1. The free app has become commercial.

2. It has become technically worse and it’s not a system preference panel any more, but an app with an annoying icon in the menu bar (maybe you can switch it off in a new version).

I don’t see any additional features compared to 1.2. It still seems to raise notifications on the screen (or speaks or sends SMS text messages), and that’s it. SO there’s no need to hurry for an upgrade, and  many users still stick to Growl 1.2.2. I just saw on McUpdate that ther is a free fork of Growl <http://www.macupdate.com/app/mac/41038/growl-fork>. So please please stay or become (in case of Thunderbird) compatible with 1.2.2 and Growl Fork!

Updated

5 years ago
status-firefox11: --- → fixed
People who are running Growl 1.2.2 and are having problems with *current Firefox mozilla-central nightlies*, please don't keep commenting here.  The bug you want is (presumably) bug 711822.
status-firefox11: fixed → ---
status-firefox11: --- → fixed
People who are running Growl 1.3 (current version 1.3.3) should not be having any problems with current Firefox trunk nightlies.

If you are, please open a new bug.  And I do mean trunk nightlies -- not alphas or betas.
Whiteboard: [inbound] → [inbound][qa+]
I'm not having any issues with Growl 1.2.2 nor 1.3.3 on Firefox 11.0b5 -- marking this VERIFIED.
Status: RESOLVED → VERIFIED
status-firefox11: fixed → verified
Whiteboard: [inbound][qa+] → [inbound][qa!]
It doesn't work for me with Firefox 11 or Thunderbird 11 Final on Mac OS X 10.7.3 with Growl 1.3.3.
Both programs aren't registered in Growl.
How can I fix that?
Thanks

Comment 37

5 years ago
I can confirm that Firefox Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0) Gecko/20100101 Firefox/11.0 DOES register correctly, but Thunderbird Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0) Gecko/20120313 Thunderbird/11.0 DOES NOT register.

Since this but is Firefox only it seems like it should not be re-opened, but see Bug 719485.

Comment 38

5 years ago
Added Bug 737444 -- Thunderbird 11 does not work with Growl 1.3.3 or 1.2.2f1. Firefox bug (this bug) remains RESOLVED WORKSFORME.
You need to log in before you can comment on or make changes to this bug.