Closed Bug 522511 Opened 11 years ago Closed 11 years ago

Update bundled Growl to 1.2 (for nsIAlertsService on OS X)

Categories

(Toolkit :: XUL Widgets, defect)

All
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta2-fixed
status1.9.1 --- ?

People

(Reporter: smichaud, Assigned: smichaud)

References

Details

(Keywords: verified1.9.2)

Attachments

(1 file, 1 obsolete file)

Some files from the Growl source distro are bundled in
toolkit/components/alerts/src/mac/growl, to provide support for
nsIAlertsService on OS X.  These files are from various places in the
Growl distro, including the following directories -- Framework/Source,
Common/Source and Core/Source.  A few contain some very minor
modifications from the original Growl code.

On the 1.9.1 and 1.9.2 branches and the trunk, these files
(originally) came from the Growl 1.1.4 distro (see bug 421551) On the
1.9.0 branch they came (as best I can tell) from the Growl 0.7 distro
(see bug 362685).  Growl source has a browseable hg repository at
http://code.google.com/p/growl/source/browse/.

The bundled Growl files need to be upgraded to version 1.2 to fix bug
510740 and bug 515396.  I'll post a patch in my next comment.
I don't recall us making any modifications to the source - what are those?
I had to add the following line to GrowlPathUtilities.mm.  So did
dtownsend when he upgraded to 1.1.4.

  #import <Cocoa/Cocoa.h>

Growl 0.7's CFGrowlAdditions.c also needed a small change to compile
on OS X 10.5 (see bug 384162).

There may be other changes that I missed, but I doubt they're many.
Here's my patch.  I just replaced all existing Growl files in
toolkit/components/alerts/src/mac/growl with newer versions that I
found in the Growl 1.2 source distro's Framework/Source, Common/Source
and Core/Source directories.  I also (as I mentioned above) made one
small edit to GrowlPathUtilities.mm.

For the record, I got the Growl 1.2 source distro from
http://growl.googlecode.com/files/Growl-1.2-src.tbz.

A tryserver build should follow in an hour or so.
Assignee: nobody → smichaud
Status: NEW → ASSIGNED
Attachment #406518 - Flags: review?(sdwilsh)
> Growl hides the source archives for versions earlier than the
> current one.

Actually this isn't quite true.  Older versions aren't accessible
following links from http://www.growl.info.  But the following URL
works (the contents of the directory aren't hidden):

http://growl.info/files/source/
Older Growl versions' binaries are available at http://growl.info/files/.
Blocks: 510740, 515396
I just realized that we can only do this upgrade on trunk.

On the branches we need to preserve compatibility with OS X 10.4, and Growl claims that Growl 1.1.6 is needed for that.
Summary: Update bundled Growl to 1.2 (for nsIAlertsService on OS X) → Update bundled Growl to 1.2 on trunk (for nsIAlertsService on OS X)
Version: unspecified → Trunk
I guess that means that on branch we'll have to backport their fix?
I suppose so.  I'll play with that tomorrow.
Attachment #406518 - Flags: review?(sdwilsh) → review+
Comment on attachment 406518 [details] [diff] [review]
Patch (upgrade Growl to version 1.2)

r=sdwilsh

Think we can get the minor changes upstream by chance to make future upgrades easier?
> I just realized that we can only do this upgrade on trunk.

I've found out this is wrong.

> On the branches we need to preserve compatibility with OS X 10.4,
> and Growl claims that Growl 1.1.6 is needed for that.

What we bundle of Growl 1.2 works just fine on OS X 10.4.11.  All
that's needed is one additional edit, in GrowlPreferencesController.h:

 @interface GrowlPreferencesController : GrowlAbstractSingletonObject {
-       LSSharedFileListRef loginItems;
+       void *loginItems;
 }

LSSharedFileListRef is new with OS X 10.5 (along with other stuff
defined in LSSharedFileList.h).  But none of this is used in the files
we bundle, which contain no other references to the loginItems member
variable of the GrowlPreferencesController class.

So all that's needed is to use 'void *' in place of the undefined
'LSSharedFileListRef'.

I'll post a new patch in my next comment, and ask you (Shawn) to
review.  I suppose we should use my new patch everywhere -- on trunk
and all the branches.
Summary: Update bundled Growl to 1.2 on trunk (for nsIAlertsService on OS X) → Update bundled Growl to 1.2 (for nsIAlertsService on OS X)
Version: Trunk → unspecified
I should note that I tested my previous comment's changes in Namoroka, on OS X 10.6.1, 10.5.8 and 10.4.11, using my STR from bug 515396 comment #12.  I didn't see any problems.
(In reply to comment #12)
>  @interface GrowlPreferencesController : GrowlAbstractSingletonObject {
> -       LSSharedFileListRef loginItems;
> +       void *loginItems;
>  }
If that's all your are changing, I don't need to review it and my previous review stands.
OK, thanks.
Landed on trunk (mozilla-central):
http://hg.mozilla.org/mozilla-central/rev/012ea897d108

I'd like to let this bake for a few days before landing on any
branches.  Since this bug blocks at least one 1.9.2-branch blocker, I
should already have approval to land there.  When I land on the 1.9.2
branch I'll request approval for the other branches.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Steven - any fallout from this bug on central? As you say, it blocks a blocker (bug 510740), so it would be nice to get it landed on 1.9.2.

Also flagging this bug blocking1.9.2+ - bugs which block blockers, block.
Flags: blocking1.9.2+
> any fallout from this bug on central?

Not that I've heard.

I'll land this on the 1.9.2 branch today.  That should give us more testers, and more time before the RC.
This patch modified the license for Growl, but no such update was made to the license for this code under about:license. In the future, please make sure you either update about:license or file a blocker bug to have it updated.
(In reply to comment #21)

Sorry.  I didn't know about this ... but now I do.
Steven: don't stress about it :-) Bug 523720 has it covered.

Gerv
Can we get this patch on 1.9.1 too? I know that is no security or stability issue but with the current behavior under 10.6 the browser window will always open in the background and our Mozmill tests fail because we need the window in the foreground.
status1.9.1: --- → ?
Target Milestone: --- → mozilla1.9.3a1
Given by bug 0 we needed this upgrade to fix bug 510740 and bug 515396. Both issues have been verified fixed on trunk and 1.9.2. That means the update of the Growl bundle was successful. Thanks Steven.
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.