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

RESOLVED FIXED in seamonkey2.21

Status

defect
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

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

Tracking

Trunk
seamonkey2.21
Dependency tree / graph

SeaMonkey Tracking Flags

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

Details

Attachments

(2 attachments)

Assignee

Description

6 years ago
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.
Assignee

Updated

6 years ago
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
Assignee

Comment 1

6 years ago
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)
Assignee

Comment 2

6 years ago
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+
Assignee

Updated

6 years ago

Updated

6 years ago
Attachment #749385 - Flags: review?(iann_bugzilla) → review+
Assignee

Comment 3

6 years ago
Thanks for the reviews, push for comm-central please.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/1c04fc3a99c6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.21
Assignee

Comment 5

6 years ago
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 6

6 years ago
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+
Assignee

Comment 7

6 years ago
Oops, I forgot about my own comment #2, will do.
Assignee

Comment 8

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

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [c-n: comm-aurora, attachment #754900]
Assignee

Updated

6 years ago
Attachment #749385 - Attachment description: Proposed patch → Proposed patch [pushed to c-c, comment #4]
Assignee

Updated

6 years ago
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.