The default bug view has changed. See this FAQ.

Add a scriptable way to set a dock badge text

RESOLVED FIXED in mozilla11

Status

()

Core
Widget: Cocoa
--
enhancement
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla11
x86
Mac OS X
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
Attachment #569771 - Flags: review?(joshmoz)

Comment 1

5 years ago
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]
Attachment #569771 - Flags: review?(joshmoz) → review-
(Assignee)

Comment 2

5 years ago
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.
Assignee: nobody → florian
Attachment #569771 - Attachment is obsolete: true
Attachment #576597 - Flags: review?(smichaud)
Attachment #576597 - Flags: review?(joshmoz)

Updated

5 years ago
Attachment #576597 - Flags: review?(joshmoz) → review+
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".
Attachment #576597 - Flags: review?(smichaud) → review+
(Assignee)

Comment 4

5 years ago
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.
Attachment #576597 - Attachment is obsolete: true
Attachment #578824 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
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.
Attachment #578824 - Flags: review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4eee92684cf
Keywords: checkin-needed
Target Milestone: --- → mozilla11
(Assignee)

Comment 7

5 years ago
(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).
https://hg.mozilla.org/mozilla-central/rev/f4eee92684cf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
Adding the dev-doc-needed keyword as I think nsIMacDockSupport.badgeText should be documented for add-on/application developers.
Keywords: dev-doc-needed
This now lives inside BlueGriffon and helped me get rid of some parts of my
binary components. Thanks a lot Florian.
Documented:

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

Updated Firefox 11 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.