Closed Bug 872133 Opened 11 years ago Closed 11 years ago

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

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
minor

Tracking

(seamonkey2.19 unaffected, seamonkey2.20 fixed, seamonkey2.21 fixed)

RESOLVED FIXED
seamonkey2.21
Tracking Status
seamonkey2.19 --- unaffected
seamonkey2.20 --- fixed
seamonkey2.21 --- fixed

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

Details

Attachments

(2 files)

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.
Summary: Focus textbox for seconds when checking "Show an alert" in Notifications preference pane → Focus textbox for "seconds" when checking "Show an alert" in Notifications preference pane
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.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attachment #749385 - Flags: ui-review?(neil)
Attachment #749385 - Flags: review?(iann_bugzilla)
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.
Attachment #749385 - Flags: ui-review?(neil) → ui-review+
Attachment #749385 - Flags: review?(iann_bugzilla) → review+
Thanks for the reviews, push for comm-central please.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/1c04fc3a99c6
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.21
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
Attachment #749385 - Flags: approval-comm-aurora?
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.
Attachment #749385 - Flags: approval-comm-aurora? → approval-comm-aurora+
Oops, I forgot about my own comment #2, will do.
Without string modifications in pref-notifications.dtd.
See attachment 749385 [details] [diff] [review] for approvals.
Keywords: checkin-needed
Whiteboard: [c-n: comm-aurora, attachment #754900]
Attachment #749385 - Attachment description: Proposed patch → Proposed patch [pushed to c-c, comment #4]
Attachment #754900 - Attachment description: Patch for comm-aurora → Patch for comm-aurora [comment #9]
You need to log in before you can comment on or make changes to this bug.