osx: make use of the 10.8+ user notification center to display growl lik... #2615

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:mac10_8_not_center changing 5 files +128 −21

jonasschnelli commented at 3:08 PM on May 3, 2013: contributor
...e notifications
- if 10.8, use user notification center, if <10.8, use growl
why:
- Growl is since 2.0 no longer free available (needs to be purchased for ~2$) and i think bitcoin should have at least a 2nd way for users who want to use free software or a free alternative (notification center is includes in 10.8+)
jonasschnelli commented at 3:09 PM on May 3, 2013: contributor

that's how it looks.
jonasschnelli commented at 3:11 PM on May 3, 2013: contributor

It would also be possible to add a settings flag where the user can switch between growl and 10.8+ user notification center.
laanwj commented at 4:14 PM on May 3, 2013: member

Don't add an option for trivial stuff such as this, if you think this looks better or is more useful (as you say, it's integrated and free), go for it.
gavinandresen commented at 4:23 PM on May 3, 2013: contributor

I vaguely recall us having issues with growl notifications in the past. Did you test this on OSX 10.5 ?
jonasschnelli commented at 6:44 PM on May 3, 2013: contributor

@gavinandresen No. It's not yet tested on 10.5 to 10.7. I'll do that in the next days. I also fix the pull-tester-report issue.

jonasschnelli commented at 10:30 AM on May 8, 2013: contributor

@gavinandresen getting crazy while building on 10.6/32bit. :) After a proper qmake/make and fancy build (dmg) i try to run Bitcoin-Qt on 10.5 but get:

dyld: Library not loaded: [@executable](/bitcoin-bitcoin/contributor/executable/)_path/../Frameworks/libssl.1.0.0.dylib
  Referenced from: /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
  Reason: no suitable image found.  Did find:
    /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/../Frameworks/libssl.1.0.0.dylib: unknown required load command 0x80000022
    /Volumes/Bitcoin-Qt 1/Bitcoin-Qt.app/Contents/MacOS/../Frameworks/libssl.1.0.0.dylib: unknown required load command 0x80000022
Trace/BPT trap

libssl looks okay: lipo -info Contents/Frameworks/libssl.1.0.0.dylib Architectures in the fat file: Contents/Frameworks/libssl.1.0.0.dylib are: i386 x86_64

any ideas?

jonasschnelli commented at 8:22 PM on May 10, 2013: contributor

puh.. had to grab out a old mac mini to do 10.6/32bit builds. Testes on 10.5 (VM) and 10.6. After some changes it worked perfect.
BitcoinPullTester commented at 8:47 PM on May 10, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b5c1406fd043bcb7d4587ad9494d1d8bca70c29c for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
Diapolo commented at 10:41 AM on May 11, 2013: none

Is this also Qt5 compatible :)?
jonasschnelli commented at 10:54 AM on May 11, 2013: contributor

@Diapolo yes. I initially wrote this patch for Qt5 compatibility because it can handle growl without qt platform functions.
laanwj commented at 5:54 PM on May 19, 2013: member

ACK (on code changes, cannot test Mac changes)

osx: make use of the 10.8+ user notification center to display growl like notifications

- if 10.8, use user notification center, if <10.8, use growl

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

b4b0170595

in src/qt/macnotificationhandler.mm:None in b5c1406fd0 outdated
```
  29 | +        [userNotification release];
  30 | +    }
  31 | +}
  32 | +
  33 | +// sendAppleScript just take a QString and executes it as apple script
  34 | +void MacNotificationHandler::sendAppleScript(const QString &script) {
```
Diapolo commented at 6:02 PM on May 19, 2013:

Can you also use our style here please :).

<pre> void MacNotificationHandler::sendAppleScript(const QString &script) { </pre>

jonasschnelli commented at 6:04 PM on May 19, 2013:

oops. yes. will change it.

in src/qt/macnotificationhandler.mm:None in b5c1406fd0 outdated

  41 | +    [as executeAndReturnError:&err];
  42 | +    [as release];
  43 | +    [scriptApple release];
  44 | +}
  45 | +
  46 | +bool MacNotificationHandler::hasUserNotificationCenterSupport(void) {

Diapolo commented at 6:03 PM on May 19, 2013:

Same as above...

in src/qt/macnotificationhandler.mm:None in b5c1406fd0 outdated

   5 | +
   6 | +void MacNotificationHandler::showNotification(const QString &title, const QString &text)
   7 | +{
   8 | +    // check if users OS has support for NSUserNotification
   9 | +    if(this->hasUserNotificationCenterSupport()) {
  10 | +        // okay, seams like 10.8+

fanquake commented at 7:50 AM on May 20, 2013:

nit: *seems

jonasschnelli commented at 7:50 AM on May 20, 2013:

thanks! my english... grml. Will fix.

jonasschnelli commented at 10:53 AM on May 24, 2013: contributor

fixed some minor code-style related things. @fanquake can you do a test on mac and give a final ACK? other final ACKs?
BitcoinPullTester commented at 11:48 AM on May 24, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b4b017059514ed0157877984363ed94f5ab098e9 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
fanquake commented at 10:11 AM on May 25, 2013: member

@jonasschnelli I'll test it this weekend
laanwj referenced this in commit a2d2e5e466 on May 30, 2013
laanwj merged this on May 30, 2013
laanwj closed this on May 30, 2013
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#5300 [Qt, OSX] fix usage of osx 10.8+ user notification center