Last Comment Bug 697546 - Add a scriptable way to set a dock badge text
: Add a scriptable way to set a dock badge text
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: x86 Mac OS X
: -- enhancement (vote)
: mozilla11
Assigned To: Florian Quèze [:florian] [:flo]
:
: Markus Stange [:mstange]
Mentors:
Depends on:
Blocks: Instantbird
  Show dependency treegraph
 
Reported: 2011-10-26 13:26 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-03-24 06:33 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.42 KB, patch)
2011-10-26 13:26 PDT, Florian Quèze [:florian] [:flo]
jaas: review-
Details | Diff | Splinter Review
Patch v2 (2.89 KB, patch)
2011-11-23 12:43 PST, Florian Quèze [:florian] [:flo]
jaas: review+
smichaud: review+
Details | Diff | Splinter Review
Patch v3 (2.90 KB, patch)
2011-12-03 04:02 PST, Florian Quèze [:florian] [:flo]
no flags Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2011-10-26 13:26:45 PDT
Created attachment 569771 [details] [diff] [review]
Patch

The code in the attached patch has been used for a while in Instantbird (and is a direct adaptation of some code living in comm-central for Thunderbird at http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessengerOSXIntegration.mm#641).

As doing binary XPCOM components is difficult for add-on authors, and slowly being deprecated, I think this code should rather be in libxul.

I don't know if I should have changed the interface uuid (do we still need to do that now that interfaces are all unfrozen and that each new version requires recompiling all binary components?). Also, I don't see any useful way to write a test for this.
Comment 1 Josh Aas 2011-11-23 08:35:06 PST
Comment on attachment 569771 [details] [diff] [review]
Patch

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

Thanks for the patch. Needs a couple of fixes then I'll r+ it, but please also request review from Steven Michaud.

::: widget/public/nsIMacDockSupport.idl
@@ +63,5 @@
> +
> +  /**
> +   * Text used to badge the dock tile.
> +   */
> +  attribute AString badgeText;

Yes, change the iid for the interface.

::: widget/src/cocoa/nsMacDockSupport.mm
@@ +81,5 @@
> +  mBadgeText = aBadgeText;
> +  if (aBadgeText.IsEmpty())
> +    [tile setBadgeLabel: nil];
> +  else
> +    [tile setBadgeLabel:[NSString stringWithFormat:@"%S", mBadgeText.get()]];

This is an inefficient way to get an NSString object for your Gecko string. You're not really taking advantage of the formatting. You probably want to do something like this:

[NSString stringWithCString:mBadgeText.get() encoding:NSUTF16StringEncoding]
Comment 2 Florian Quèze [:florian] [:flo] 2011-11-23 12:43:41 PST
Created attachment 576597 [details] [diff] [review]
Patch v2

(In reply to Josh Aas (Mozilla Corporation) from comment #1)

> [NSString stringWithCString:mBadgeText.get() encoding:NSUTF16StringEncoding]

This didn't work, a const char* argument is expected but mBadgeText.get() is PRUnichar *. I used stringWithCharacters instead, I saw it's used in several places in the Mozilla code already.
Comment 3 Steven Michaud [:smichaud] (Retired) 2011-12-01 08:10:31 PST
Comment on attachment 576597 [details] [diff] [review]
Patch v2

> +  id tile

'tile' should be an NSDockTile pointer.  Other than that your patch looks fine.

But please attach a minimal extension that uses your new 'badgeText' attribute, so that your patch can at least be tested "by hand".
Comment 4 Florian Quèze [:florian] [:flo] 2011-12-03 04:02:38 PST
Created attachment 578824 [details] [diff] [review]
Patch v3

Review comment addressed. Carrying forward the r+.

To test this by hand, the easiest way is to paste this in the error console:

Components.classes["@mozilla.org/widget/macdocksupport;1"].getService(Components.interfaces.nsIMacDockSupport).badgeText="foo";

and check that a badge with the text "foo" appears on the dock icon.

Then, paste:

Components.classes["@mozilla.org/widget/macdocksupport;1"].getService(Components.interfaces.nsIMacDockSupport).badgeText="";

and check that the badge disappeared.
Comment 5 Dão Gottwald [:dao] 2011-12-05 01:56:45 PST
Comment on attachment 578824 [details] [diff] [review]
Patch v3

When using checkin-needed, it would be ideal if you could add a commit message including reviewers to your patch. However, if you don't do this, please at least don't set the review flag yourself, as this may confuse people as to who reviewed this.
Comment 7 Florian Quèze [:florian] [:flo] 2011-12-05 02:19:17 PST
(In reply to Dão Gottwald [:dao] from comment #5)

> When using checkin-needed, it would be ideal if you could add a commit
> message including reviewers to your patch. However, if you don't do this,
> please at least don't set the review flag yourself, as this may confuse
> people as to who reviewed this.

Sorry about that, and thanks for the check-in. I didn't intend to use checkin-needed at the time I attached the patch and set the flag, but I then noticed that my hg account is disabled (bug 707468).
Comment 8 Matt Brubeck (:mbrubeck) 2011-12-05 10:40:31 PST
https://hg.mozilla.org/mozilla-central/rev/f4eee92684cf
Comment 9 Florian Quèze [:florian] [:flo] 2011-12-08 16:20:17 PST
Adding the dev-doc-needed keyword as I think nsIMacDockSupport.badgeText should be documented for add-on/application developers.
Comment 10 Daniel Glazman (:glazou) 2011-12-14 00:12:33 PST
This now lives inside BlueGriffon and helped me get rid of some parts of my
binary components. Thanks a lot Florian.
Comment 11 Eric Shepherd [:sheppy] 2012-03-24 06:33:55 PDT
Documented:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIMacDockSupport

Updated Firefox 11 for developers.

Note You need to log in before you can comment on or make changes to this bug.