1700440 - Update the preferences dialog to be inline with the changes in bug 715799

Status: RESOLVED FIXED

(Thunderbird :: Preferences, task)

Description

[Richard Marti (:Paenglab)]

After bug 715799 on Windows there is now the possibility to show a badge on the taskbar icon (and a icon with a red dot in the tray). Also the system notification can be used instead of the TB new mail alert.

We need now probably a mix of the actual prefs and the Mac prefs for Windows.

Ping, could you describe exactly what is possible now (including the prefs names) to know what should come into the prefs UI?

Then hopefully Alessandro can design the needed prefs and/or a dialog.

Comment 1

[Ping Chen [:rnons] (away)]

Prefs newly exposed to Windows

• mail.biff.use_system_alert, but the system alert doesn't work on Win7, not sure how to show it in pref ui. Mac always uses system alert so doesn't have this.
• mail.notification.count.inbox_only, by default the badge shows unread count of Inbox only and chat messages... The relevant code is quite old, I don't think we have a pref to control counting of chat/feed. We might want to add those prefs and UI later. The feed part relates to bug 261841

Prefs got new meaning on Windows

• mail.biff.show_tray_icon, previously means new messages arrive. Now means when unread count > 0, a tray icon with a red dot is shown.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: 1700440-new-biff-options.patch

Okay, how about this?

I exposed the biff-use-system-alert for Linux too.
I renamed the text of mail.biff.show_tray_icon. See following screenshot.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: newMailSettings.png

How it looks on Windows.

Comment 4

[Alessandro Castellani [:aleca]]

Comment on attachment 9212041 [details] [diff] [review]
1700440-new-biff-options.patch

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

Good stuff.
It needs some small UI changes.
Also, the commit message has a curly bracket at the beginning. We should maybe update it with:
`[Windows] Expose the new biff-use-system-alert options.`

::: mail/components/preferences/general.inc.xhtml
@@ +442,5 @@
> +                  data-l10n-id="tray-icon-unread-label"/>
> +        <description class="indent tip-caption"
> +                     flex="1"
> +                     data-l10n-id="tray-icon-unread-description"/>
> +      </hbox>

Let's use the same UI approach we have here: https://searchfox.org/comm-central/rev/51ad486532b07f7a4f1ad64335045a46fe50de83/mail/extensions/am-e2e/am-e2e.inc.xhtml#193-197

The description should be indented underneath the checkbox.

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +216,3 @@
>      .accesskey = t
>  
> +tray-icon-unread-description = Useful with small taskbar buttons

More than "Useful" I'd use "Recommended", what do you think?
Also, are those application tray icon called "buttons" in Windows? I'm not sure if this is super clear.

::: mail/themes/shared/mail/preferences/preferences.css
@@ +158,5 @@
>    overflow: hidden;
>    text-overflow: ellipsis;
>  }
>  
> +.tip-caption {

We already have a tip-caption class in the declared a bunch of times around different CSS files.
Can we unify it?
https://searchfox.org/comm-central/search?q=.tip-caption&path=&case=true&regexp=false

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1700440-new-biff-options.patch

(In reply to Alessandro Castellani [:aleca] from comment #4)

It needs some small UI changes.
Also, the commit message has a curly bracket at the beginning. We should
maybe update it with:
[Windows] Expose the new biff-use-system-alert options.

done

::: mail/components/preferences/general.inc.xhtml
@@ +442,5 @@

•              data-l10n-id="tray-icon-unread-label"/>
  

•    <description class="indent tip-caption"
  

•                 flex="1"
  

•                 data-l10n-id="tray-icon-unread-description"/>
  

•  </hbox>
  

Let's use the same UI approach we have here:
https://searchfox.org/comm-central/rev/
51ad486532b07f7a4f1ad64335045a46fe50de83/mail/extensions/am-e2e/am-e2e.inc.
xhtml#193-197

The description should be indented underneath the checkbox.

done

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +216,3 @@

 .accesskey = t

+tray-icon-unread-description = Useful with small taskbar buttons

More than "Useful" I'd use "Recommended", what do you think?
Also, are those application tray icon called "buttons" in Windows? I'm not
sure if this is super clear.

They call it "buttons".

::: mail/themes/shared/mail/preferences/preferences.css
@@ +158,5 @@

overflow: hidden;
text-overflow: ellipsis;
}

+.tip-caption {

We already have a tip-caption class in the declared a bunch of times around
different CSS files.
Can we unify it?
https://searchfox.org/comm-central/search?q=.tip-
caption&path=&case=true&regexp=false

Removed the rule from accountManage.css as it uses the one from preferences.css.

Comment 6

[Richard Marti (:Paenglab)]

Comment on attachment 9212291 [details] [diff] [review]
1700440-new-biff-options.patch

Sorry thought you gave a r+ with comments.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9212291 [details] [diff] [review]
1700440-new-biff-options.patch

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

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +207,5 @@
>  
> +biff-use-system-alert =
> +    .label = { PLATFORM() ->
> +        [windows] Use the system notification
> +        *[other] Use the system alert

Well, on linux is system notification  as well. 
And shouldn't this be all hidden on mac since notification center is the only option there. Or?

@@ +216,3 @@
>      .accesskey = t
>  
> +tray-icon-unread-description = Recommended with small taskbar buttons

I think "with" is not correct English here. It's "when you're using"

Comment 8

[Richard Marti (:Paenglab)]

Attachment: 1700440-new-biff-options.patch

(In reply to Magnus Melin [:mkmelin] from comment #7)

Comment on attachment 9212291 [details] [diff] [review]
1700440-new-biff-options.patch

Review of attachment 9212291 [details] [diff] [review]:

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +207,5 @@

+biff-use-system-alert =

• .label = { PLATFORM() ->

•    [windows] Use the system notification
  

•    *[other] Use the system alert
  

Removed the platform switch.

Well, on linux is system notification as well.
And shouldn't this be all hidden on mac since notification center is the
only option there. Or?

It's in the #else block of #ifdef XP_MACOSX and only shown on Linux and Windows.

@@ +216,3 @@

 .accesskey = t

+tray-icon-unread-description = Recommended with small taskbar buttons

I think "with" is not correct English here. It's "when you're using"

done

Comment 9

[Alessandro Castellani [:aleca]]

Comment on attachment 9212307 [details] [diff] [review]
1700440-new-biff-options.patch

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

Looks good, with a little nit on the string.

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +213,3 @@
>      .accesskey = t
>  
> +tray-icon-unread-description = Recommended when you're using small taskbar buttons

No need to indicate the subject here.
"Recommended when using small taskbar buttons" sounds more correct.

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1700440-new-biff-options.patch

Updated the description.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3c5e687f409b
[Windows] Expose the new biff-use-system-alert options. r=aleca

Comment 12

[Richard Marti (:Paenglab)]

Comment on attachment 9212314 [details] [diff] [review]
1700440-new-biff-options.patch

[Approval Request Comment]
User impact if declined: the user can't set the options approprately
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9212314 [details] [diff] [review]
1700440-new-biff-options.patch

[Triage Comment]
Approved for beta

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 88.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/0b3bf4b512c3

Comment 15

[Addons]

Just out of interest, has the rule not to uplift things with string changes been abandoned?
https://hg.mozilla.org/releases/comm-beta/diff/0b3bf4b512c3e8934208eb48ab7d818e3a7ff69e/mail/locales/en-US/messenger/preferences/preferences.ftl

Comment 16

[Magnus Melin [:mkmelin]]

Rule still in affect.

Comment 17

[Magnus Melin [:mkmelin]]

For 78 at least. For beta, I guess we should generally stick to it as well.