Closed Bug 697546 Opened 8 years ago Closed 8 years ago

Add a scriptable way to set a dock badge text

Categories

(Core :: Widget: Cocoa, enhancement)

x86
macOS
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: florian, Assigned: florian)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

Attached patch Patch (obsolete) — Splinter Review
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 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-
Attached patch Patch v2 (obsolete) — Splinter Review
(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)
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+
Attached patch Patch v3Splinter Review
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+
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+
(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
Closed: 8 years ago
Resolution: --- → FIXED
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.
You need to log in before you can comment on or make changes to this bug.