872133 - Focus textbox for "seconds" when checking "Show an alert" in Notifications preference pane

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[rsx11m]

This is another follow-up to bug 856454, for the design of the extended "Show an alert for [__] seconds" option. The current implementation (a) doesn't focus the textbox for "seconds" and (b) has a separate accesskey for that label.

Per bug 867210 comment #17 (and those leading up to it), no other preference pane shows this behavior; the preferred design implies that checking the box automatically focuses the textbox, thus the accesskey is shared between those to UI elements.

Comment 1

[rsx11m]

Attachment: Proposed patch [pushed to c-c, comment #4]

This patch ports the logic from bug 867210 to focus showAlertTime when EnableAlert() is invoked by a change mail.biff.show_alert towards a "true" value.

I'm also taking care of bug 869116 for this instance while I'm there, thus graying "seconds" in sync with the "Show an alert for" label.

Comment 2

[rsx11m]

BTW: The patch would be suitable for comm-aurora without the changes to pref-notifications.dtd (removing showAlertTimeEnd.accesskey, which isn't necessary for the modifications in pref-notifications.{js,xul} to work). Thus, once approved, I'll post a branch patch without the locale/ changes so that there won't be a different interim behavior just for the 2.20 release.

Comment 3

[rsx11m]

Thanks for the reviews, push for comm-central please.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/1c04fc3a99c6

Comment 5

[rsx11m]

Comment on attachment 749385 [details] [diff] [review]
Proposed patch [pushed to c-c, comment #4]

[Approval Request Comment]
Regression caused by (bug #): Follow-up fix to bug 856454
User impact if declined: SeaMonkey 2.20 would have a different behavior / UX
Testing completed (on m-c, etc.): tested and works fine with 2.20
Risk to taking this patch (and alternatives if risky): low
String changes made by this patch: none

Comment 6

[Ian Neal]

Comment on attachment 749385 [details] [diff] [review]
Proposed patch [pushed to c-c, comment #4]

There is one string change, but the patch still works without that change. Please generate a patch for checkin to comm-aurora without that string change.

Comment 7

[rsx11m]

Oops, I forgot about my own comment #2, will do.

Comment 8

[rsx11m]

Attachment: Patch for comm-aurora [comment #9]

Without string modifications in pref-notifications.dtd.
See attachment 749385 [details] [diff] [review] for approvals.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/comm-aurora/rev/485de398bd2f