1106815 - Always show alerts on OS X 10.8+ and let user setup them on Notification Center

Status: RESOLVED FIXED

(Thunderbird :: OS Integration, defect)

Description

[Javi Rueda :javirid]

https://developer.apple.com/library/mac/documentation/UserExperience/Conceptual/OSXHIGuidelines/NotificationCenter.html

Since OS X 10.8, user can enable and disable alerts (and badges) from Notification Center. Thunderbird is using, however a custom UI for enabling those, through the Preference window.

What I am proposing here is to show always an alert, whatever the TB prefderence is set to, and let user enable or disable them on System Preferences/Notifications.

This bug could maybe be moved to some MailNews component, as Notification alerts code is in mailnews/

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessengerOSXIntegration.mm#399

Comment 1

[Javi Rueda :javirid]

Bug 391674 is about allowing the user to not shown the badge on the icon. Currently, I am directing users complaining about this to set it up on the Notification settings (on System Preferences). However, some of them are noticing that their is no "Thunderbird" entry on that section.

That could be caused because thay have disabled "Send an alert" on the General pane on TB settings. If no alert has ever been sent, Thunderbird hasn't been registered on that section on System Preferences.

That problem is the one this bug shopuld try to fix. And the way I think it should be is just to show always an alert if we are on a device running OS X 10.8. Then user could go to Notification section on System PReferences and then select "None" as the notification type which would be the same as unchecking "Show an alert" on the preferences window on TB.

Once this bug is fixed, I would disable UI options when needed on TB and provide a label directing user to System Preferences to change this behavior.

However, this bug could finally be set to WONTFIX. That is why I have not open the corresponding bug on Thunderbird::Preferences.

It is highly probable that Seamonkey could be affected by this problem also, however I almost have no experience on the way they are doing things.

Comment 2

[Javi Rueda :javirid]

Attachment: patch

From OS X 10.8, users should manage their notification preferences from the Notification Center. App will not be shown there if we have not post at least one notification (this could happen if user disabled alerts from the Preferences dialog). By bypassing that preference, app will always be registered on the Notification Center.

A follow-up bug should then direct users to Notification Center to set those preferences.

Could you review this patch, Markus? Thank you.

Comment 3

[Richard Marti (:Paenglab)]

OS X 10.7 and older are deprecated. Is bMountainLionOrLater still needed?

Comment 4

[Javi Rueda :javirid]

I think it is a bit confusing. Bug 1279017 was intended to remove code for OS X 10.7 and below. But there are some code lines which, I am thinking, allow the Build Team to build on OS X 10.7 (which, are really using, as it is shown in TreeHerder).

Then, there is the bug 1279017 comment 1, which says Core component does not support OS X 10.8 and below (as you are saying, :Paenglab).

However, looking into "System Requirements" for Thunderbird, it is displayed OS X 10.6.

I am probably wrong; it has been I while since I posted a patch for Thunderbird for last time. In that case, I could redo the patch and remove any code about detecting OS X 10.8. Then, the if-conditional could be completely removed.

Comment 5

[Javi Rueda :javirid]

Attachment: Remove usage of show_alert preference

This patch removes the usage of the mail.biff.show_alert as now this should be managed throughout native OS X Notification Center as recommended by Apple Human Interface Guidelines.

Mozilla 50 deprecated OS X versions older than 10.9. Notification Center was available since OS X 10.8. So there was no need to query the OS environment which we were running on.

Firefox is deprecating OS X 10.8- but I have not found any evidence that Thunderbird nor SeaMonkey was doing the same already. That is the reason I am not obsoleting the old patch in this bug.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 8767317 [details] [diff] [review]
Remove usage of show_alert preference

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

Thunderbird follows whatever platform Firefox supports, so this looks good, with a small change. r=mkmelin

::: mailnews/base/src/nsMessengerOSXIntegration.mm
@@ +389,1 @@
>    NS_ENSURE_SUCCESS(rv, rv);

Remove this NS_ENSURE_SUCCESS, as you check success failure later instead

Comment 7

[Javi Rueda :javirid]

Attachment: corrected patch

Patch corrected. Removed NS_ENSURE_SUCCESS line of code and aded reviewer to patch message.

Thank you, Magnus.

Comment 8

[Javi Rueda :javirid]

I have just opened a new bug 1283936 so the UI is removed. I removed also the checkin-needed flag until that new bug is ready to land.

Comment 9

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/7afa9b753183fcd8cbf0dc35e9d45678b3af3498
Bug 1106815 - Always show alerts on OS X 10.8+ and let user setup them on Notification Center. r=mkmelin