Closed Bug 856451 Opened 7 years ago Closed 7 years ago

Make the New Mail alert show time easy adjustable

Categories

(Thunderbird :: Preferences, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 23.0

People

(Reporter: Paenglab, Assigned: Paenglab)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

To adjust the time where the New Mail notification isn't easy to do. In Options/Preferences in Advanced you have to go to general tab and then open the Config Editor. Here you have to search for alerts.totalOpenTime and then to know the value is in milliseconds.

In General 'Customize New Mail Alarm' where would be a good place. In it isn't much and a new pref doesn't make it overloaded.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #731691 - Flags: ui-review?(bwinton)
Attachment #731691 - Flags: review?(mconley)
Blocks: 856454
Comment on attachment 731691 [details] [diff] [review]
patch

Why do we want to let people easily adjust this?

I'm also not entirely sure, if we want to let people adjust it in the first place, why we would restrict it to whole seconds, and why we would restrict the range to between 1 and 99 seconds.  Surely someone out there requires the ability to show an alert for .25 seconds, or for an hour…  ;)

Based on these two questions, I'm going to say ui-r-, at least for now.
Attachment #731691 - Flags: ui-review?(bwinton) → ui-review-
(In reply to Blake Winton (:bwinton) from comment #3)
> Comment on attachment 731691 [details] [diff] [review]
> patch
> 
> Why do we want to let people easily adjust this?

Bug 831737 changed the show time from 3 seconds to 10 because the fade-in and fade-out have each a duration of 2 seconds (equal to the alerts in Firefox) and with 3 seconds show time the notification wasn't shown with full opacity. I saw already people mentioning 10 seconds are to long (and other it's still to short). When this changes comes public with TB 24 I can think of a lot of complains about this change and a easy possibility to change could damp this complaints. And as I wrote already, this option wouldn't make the dialog overloaded.

> I'm also not entirely sure, if we want to let people adjust it in the first
> place, why we would restrict it to whole seconds, and why we would restrict
> the range to between 1 and 99 seconds.  Surely someone out there requires
> the ability to show an alert for .25 seconds, or for an hour…  ;)

I could add decimalplaces="1" then it would be adjustable by tenth of seconds. I restricted only the minimum to 1 second. And I tried to set one hour and I could set it but didn't tested it with an new Mail alert ;)

I'm okay when you say WONTFIX, but maybe it's worth a try.
Blocks: 856759
Comment on attachment 731691 [details] [diff] [review]
patch

Okay, that makes sense.  I'll say ui-r=me, if we can set it to 0 (to disable the alerts), and have a maximum time of something like an hour, and be able to go up in tenths of a second.

Thanks,
Blake.
Attachment #731691 - Flags: ui-review- → ui-review+
Attached patch patch v2 (obsolete) — Splinter Review
0 doesn't disable the alerts. It is still shown for the fade-in and fade-out time. The user can disable the notification in main preferences window with unticking the checkbox '[] Show an alert'. I'll let the minimum at 1. The maximum is now on 3600.

I'm carrying over the ui-r from previous patch.
Attachment #731691 - Attachment is obsolete: true
Attachment #731691 - Flags: review?(mconley)
Attachment #732014 - Flags: ui-review+
Attachment #732014 - Flags: review?(mconley)
Tenth of a second? Really? Especially given we have fade-in/fade-outs of totally 5 secs in any case.
+1; I've tried it and I find it very hard to actually change the decimal digit. The dials go in full seconds, and you need to highlight the decimal digit (and only it) to modify the fractions with the keyboard. That's a lot of effort to go with a quarter second more or less...
(In reply to Magnus Melin from comment #7)
> fade-in/fade-outs

The user could disable those with alerts.disableSlidingEffect in the Config Editor, thus a minimum of 1sec still makes sense from that perspective (though you won't see much which such a short duration, but it's hard to define and impose a "useful" minimum).
Comment on attachment 732014 [details] [diff] [review]
patch v2

>+      <textbox id="totalOpenTime" type="number" decimalplaces="1" size="3" min="1" max="3600"

FYI: The SeaMonkey patch in bug 856454 has received ui-r+ without the decimalplaces, thus full seconds only.

>+               onsyncfrompreference="let tot = document.getElementById(this.getAttribute('preference'));
>+                                     return (tot.instantApply ? tot.valueFromPreferences : tot.value) / 1000;"

That's more complicated than apparently necessary. Per Neil's bug 856454 comment #10, you don't seem to need the check for instantApply using valueFromPreferences instead of just value. I've used the following simplification and it works fine on both Windows (applied on "Ok") and Linux (instantly) in my patch:

>+                   onsyncfrompreference="return document.getElementById(this.getAttribute('preference')).value / 1000;"
Attached patch patch v3 (obsolete) — Splinter Review
(In reply to rsx11m from comment #10)
> Comment on attachment 732014 [details] [diff] [review]
> patch v2
> 
> >+      <textbox id="totalOpenTime" type="number" decimalplaces="1" size="3" min="1" max="3600"
> 
> FYI: The SeaMonkey patch in bug 856454 has received ui-r+ without the
> decimalplaces, thus full seconds only.

I'll let Mike decide if we want to show the decimalplaces. I could also add increment="0.5" then it would always increment/decrement a half second and the tenth field isn't needed to select.

> >+                   onsyncfrompreference="return document.getElementById(this.getAttribute('preference')).value / 1000;"

Thank you for this tip. I tested it and it works. This patch has this now.
Attachment #732014 - Attachment is obsolete: true
Attachment #732014 - Flags: review?(mconley)
Attachment #732874 - Flags: ui-review+
Attachment #732874 - Flags: review?(mconley)
Comment on attachment 732874 [details] [diff] [review]
patch v3

Let's go with .5 seconds, as Richard suggested.

With that increment, r=me.
Attachment #732874 - Flags: review?(mconley) → review+
Added the increment.

Carrying over r+ and ui-r+
Attachment #732874 - Attachment is obsolete: true
Attachment #735627 - Flags: ui-review+
Attachment #735627 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/505edc73c23a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
You need to log in before you can comment on or make changes to this bug.